Fix heap over-read seeding the long-column buffer in pdo_odbc#22349
Fix heap over-read seeding the long-column buffer in pdo_odbc#22349iliaal wants to merge 1 commit into
Conversation
SakiTakamachi
left a comment
SakiTakamachi
left a comment
There was a problem hiding this comment.
Indeed. I’d also like to wait for Calvin’s review.
NattyNarwhal
left a comment
NattyNarwhal
left a comment
There was a problem hiding this comment.
This sounds right. Have you ran into this with a driver or is this static analysis?
In the long-column fetch path, when the ODBC driver reports the total column length rather than SQL_NO_TOTAL, the result string was seeded by copying orig_fetched_len + 1 bytes out of C->data, which holds at most LONG_COLUMN_BUFFER_SIZE bytes from the first SQLGetData. For a column larger than that buffer this reads past C->data. Seed only the bytes actually present in the buffer, matching the SQL_NO_TOTAL branch; the remainder is still fetched by the loop.
iliaal
force-pushed
the
fix-pdo-odbc-long-overread
branch
from
8a28a54 to
a1ee48e
Compare
|
Originally static analysis, but I've now reproduced it against a live driver. psqlODBC (PostgreSQL ANSI) returns the exact column length on the truncated SQLGetData, so a text value larger than LONG_COLUMN_BUFFER_SIZE takes the unclamped branch. With a 64 KB text column and an ASAN build, the fetch trips a heap-buffer-overflow READ of 65537 bytes out of the 4064-byte buffer in odbc_stmt_get_col. The sqlite ODBC driver returns SQL_NO_TOTAL, which is why that path stayed hidden. Added a regression test (gh22349.phpt); it skips without an ODBC DSN and fails under ASAN against psqlODBC. |
3 participants
In the long-column fetch path, when the ODBC driver reports the total column length rather than SQL_NO_TOTAL, the result string was seeded by copying orig_fetched_len + 1 bytes out of C->data, which holds at most LONG_COLUMN_BUFFER_SIZE bytes from the first SQLGetData. For a column larger than that buffer this reads past C->data. Seed only the bytes actually present, matching the SQL_NO_TOTAL branch; the remainder is still fetched by the loop.