aws / aws/amazon-redshift-odbc-driver
Buffer overread related to truncated data from `SQLFetchScroll`
- Dominant language
- C
- Stars
- 25
- Forks
- 17
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
The Redshift ODBC driver (v2.1.11+) contains a buffer overread in `libpqCreateSQLTablesCustomizedResultSet()` that causes `SQLTablesW` to fail with "Unicode conversion failed" (or crash with `SEHException` on older versions) when any table in the database has a `COMMENT` longer than 255 bytes.
Data is truncated upstream by `SQLFetchScroll` and the same function provides a length which represents the original, untruncated length (by design), which is incorrectly used downstream to copy data.
## Root Cause
In the `ShowTablesHelper::execute()` method (`rsMetadataServerProxyHelper.cpp`), the `SHOWTABLESResult` struct uses a fixed 256-byte buffer for `remarks` (`MAX_REMARK_LEN`). When `RS_SQLFetchScroll` fetches a row where the `remarks` value exceeds 255 bytes:
1. The data is **truncated** to 255 bytes + null terminator in the 256-byte buffer
2. The length indicator (`remarks_Len`) is set to the **full original length** (e.g., 1473 bytes) per ODBC specification for `SQL_SUCCESS_WITH_INFO` / `01004` (String data, right truncation)
3. The `SHOWTABLESResult` struct (with truncated buffer but full-length indicator) is stored in `m_intermediateRS`
Later, in `libpqCreateSQLTablesCustomizedResultSet()` (`rslibpq.c`), the data is transferred to a `PGresult`:
```c
PQsetvalue(res, finalRowNum, kSQLTables_REMARKS_COL_NUM,
(char *)intermediateRS[i].remarks, // 256-byte buffer
intermediateRS[i].remarks_Len); // e.g., 1473
```
`PQsetvalue` calls `memcpy(dest, source, 1473)` on a 256-byte buffer, reading 1217 bytes of uninitialized heap memory. This garbage data later fails UTF-8 to UTF-16 conversion in `copyWStrDataBigLen()` (`rsutil.c`), producing the "Unicode conversion failed" error.
## Affected Versions
- **v2.1.13**: Returns `ERROR [HY000] Unicode conversion failed`
- **v2.1.11**: Crashes with `SEHException: External component has thrown an exception`
## Evidence from Driver Trace Log
With `LogLevel=6` enabled, the trace shows:
```
[TRACE] convertSQLDataToCData ... iColDataLen=726 iConversion=1
[DEBUG] String data, right truncation occurred: Buffer too small to hold the entire data.
...
[ERROR] Unicode conversion failed. Context: pStmt=..., iSrcLen=726, cbLen=4094, cbLenOffset=0, ..., strlen(pSrc)=255
[ERROR] UTF-8→UTF-16 conversion failed: bad conversion
```
Note `iSrcLen=726` (driver thinks the string is 726 bytes) but `strlen(pSrc)=255` (actual null-terminated content is only 255 bytes due to truncation). The 471 bytes between position 255 and 726 are garbage from the heap overread.
## Affected Code Paths
The same pattern (uncapped `_Len` passed to `PQsetvalue`) applies to ALL fixed-size buffer fields in `SHOWTABLESResult`. While `remarks` (256-byte buffer) is the most likely to overflow in practice, any field exceeding its buffer size would trigger the same bug:
- `database_name`, `schema_name`, `table_name`, `table_type`, `owner`, `last_altered_time`, `last_modified_time`, `dist_style`, `table_subtype`: 128 bytes (`NAMEDATALEN`)
- `remarks`: 256 bytes (`MAX_REMARK_LEN`)
The same pattern also exists in `libpqCreateSQLColumnsCustomizedResultSet` and other `libpqCreate*CustomizedResultSet` functions for `SHOWCOLUMNSResult`, `SHOWCONSTRAINTSPRIMARYKEYSResult`, etc.
## Reproduction Steps
1. Connect to a Redshift cluster where `show_discovery >= 4`
2. Create a table with a comment longer than 255 bytes:
```sql
CREATE TABLE test_long_comment (id INT);
COMMENT ON TABLE test_long_comment IS '';
```
3. Call `SQLTablesW` (e.g., via .NET `OdbcConnection.GetSchema("Tables")`)
4. Observe: "Unicode conversion failed" error
Contributor guide
Research direction
Start in rsMetadataServerProxyHelper.cpp at ShowTablesHelper::execute and trace the fixed-size SHOWTABLESResult fields into libpqCreateSQLTablesCustomizedResultSet in rslibpq.c. Compare the related customized-result-set functions and reproduce with SQLTablesW and a comment longer than 255 bytes; done means truncated fields no longer cause buffer overreads, Unicode conversion failures, or crashes in rsutil.c.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100