aws / aws/amazon-redshift-odbc-driver

Buffer overread related to truncated data from `SQLFetchScroll`

Open
#41 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.