postgresql-interfaces/psqlodbc GitHub issues and pull requests (mirror)help / color / mirror / Atom feed
[postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals 2+ messages / 2 participants [nested] [flat]
* [postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals @ 2026-05-11 08:59 "jarvis24young (@jarvis24young)" <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: jarvis24young (@jarvis24young) @ 2026-05-11 08:59 UTC (permalink / raw) To: postgresql-interfaces/psqlodbc <[email protected]> This fixes a one-byte read before the start of the prepared SQL buffer when parameter counting sees a leading string literal while standard_conforming_strings is on. Call chain reproduced through unixODBC with an ASan build: - SQLPrepare(hstmt, "'abc'", SQL_NTS) copies the SQL into the statement buffer. - SQLNumParams(hstmt, ...) calls PGAPI_NumParams(). - PGAPI_NumParams() calls SC_scanQueryAndCountParams(). - SC_scanQueryAndCountParams() saw the leading quote, CC_get_escape() returned 0, and the E'' prefix check read ENCODE_PTR(encstr)[-1]. ASan before the fix reported: - heap-buffer-overflow READ of size 1 - statement.c:1125 in SC_scanQueryAndCountParams - address located 1 byte before the buffer allocated by make_string() from PGAPI_Prepare() The patch guards the E'' prefix check in the three scanner paths so the previous byte is read only when the current scanner position is not at the beginning of the SQL string. This preserves E'' handling and avoids changing normal literal parsing. Verification in WSL: - Built psqlodbc with -fsanitize=address,undefined - Reproduced the ASan failure before the fix with exe/leading-literal-numparams-test - Rebuilt after the fix and confirmed the same test prints nparams=0 without ASan errors - Ran the project harness: ./runsuite leading-literal-numparams --inputdir=. -> ok 1 ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals @ 2026-05-13 09:39 "davecramer (@davecramer)" <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: davecramer (@davecramer) @ 2026-05-13 09:39 UTC (permalink / raw) To: postgresql-interfaces/psqlodbc <[email protected]> Summary Fixes a heap-buffer-overflow (1-byte read before buffer start) when the SQL string begins with a string literal (e.g., 'abc') and standard_conforming_strings is on (CC_get_escape() returns 0). The scanner checks for a preceding E character (PostgreSQL's E'...' escape string syntax) by reading ptr[-1], but when the literal is at position 0, that's a read before the buffer. The Bug Three independent scanner paths have the same pattern: if (LITERAL_EXT == ptr[-1]) // reads before buffer when at position 0 Where ptr is: - ENCODE_PTR(encstr) = encstr.encstr + encstr.pos in statement.c - F_OldPtr(qp) = qp->statement + qp->opos in convert.c - tstr (advancing pointer) in parse.c The Fix Each site adds a bounds check before the [-1] access: ┌─────────────┬─────────────────────────────┐ │ File │ Guard added │ ├─────────────┼─────────────────────────────┤ │ statement.c │ encstr.pos > 0 && │ ├─────────────┼─────────────────────────────┤ │ convert.c │ qp->opos > 0 && │ ├─────────────┼─────────────────────────────┤ │ parse.c │ tstr > (const UCHAR *) s && │ └─────────────┴─────────────────────────────┘ Short-circuit evaluation ensures [-1] is never reached when at position 0. Assessment Correctness: ✅ - All three guards are correct for their respective pointer arithmetic. - encstr.pos > 0 directly maps to ENCODE_PTR(encstr)[-1] being valid since ENCODE_PTR is encstr + pos. - qp->opos > 0 directly maps to F_OldPtr(qp)[-1] being valid since F_OldPtr is statement + opos. - tstr > (const UCHAR *) s is the pointer-based equivalent for parse.c where s is the start of the string and tstr advances through it. - When at position 0, the E'' prefix check is correctly skipped — a literal at position 0 can't have a preceding E anyway. Completeness: ✅ - All three scanner paths that have this pattern are fixed. I checked — there are no other LITERAL_EXT == ...[-1] patterns in the codebase. Test: ✅ - Minimal reproducer: SQLPrepare(hstmt, "'abc'", SQL_NTS) followed by SQLNumParams(). - Expected output file included. - Test registered in test/tests. - ASan verification documented in the PR description. Risk: Very low - The fix only adds a guard that skips an impossible condition (can't have E before position 0). No behavioral change for any valid input. Verdict Textbook bounds-check fix. Correct, minimal, well-tested across all three affected sites. Merge-worthy as-is. ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-05-13 09:39 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-05-11 08:59 [postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals "jarvis24young (@jarvis24young)" <[email protected]> 2026-05-13 09:39 Re: [postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals "davecramer (@davecramer)" <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox