postgresql-interfaces/psqlodbc GitHub issues and pull requests (mirror)  

help / color / mirror / Atom feed

From: davecramer (@davecramer) <[email protected]>
To: postgresql-interfaces/psqlodbc <[email protected]>
Subject: Re: [postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals
Date: Wed, 13 May 2026 09:39:29 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[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.

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: github://postgresql-interfaces/psqlodbc
  Cc: [email protected], [email protected]
  Subject: Re: [postgresql-interfaces/psqlodbc] PR #185: Avoid reading before leading string literals
  In-Reply-To: <<[email protected]>>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox