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 #182: Reject invalid SQLPutData lengths
Date: Fri, 08 May 2026 10:01:21 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

This PR adds input validation to PGAPI_PutData() to reject invalid negative StrLen_or_Ind values that could corrupt
  internal statement state. It's a defensive hardening fix.
  
  What it does
  
  1. execute.c — Two validation checks added:
    - On first SQLPutData call: if cbValue is negative but not one of the ODBC sentinels (SQL_NULL_DATA,
  SQL_DEFAULT_PARAM), return HY090 error instead of blindly storing it as a length.
    - On subsequent calls (append path): if EXEC_used is already negative (null/default indicator), reject the append
  with HY020 instead of using a negative offset for memcpy.
  
  2. statement.h / statement.c — Adds two new error codes (STMT_INVALID_STRING_OR_BUFFER_LENGTH → HY090,
  STMT_ATTEMPT_TO_CONCATENATE_NULL_VALUE → HY020) with correct ODBC 2.x/3.x SQLSTATE mappings.
  3. test — Adds a regression test that calls SQLPutData(hstmt, "bad", -10) and verifies SQL_ERROR with SQLSTATE
  HY090.
  
  Positives
  
  - Correct ODBC semantics: HY090 (invalid string/buffer length) and HY020 (attempt to concatenate null) are the right
  SQLSTATEs per the ODBC spec.
  - Minimal and focused: Only touches the paths that need validation.
  - Good test coverage: The test exercises the exact failure path and validates the diagnostic record.
  - Prevents real corruption: Without this fix, a negative cbValue stored in EXEC_used becomes a negative old_pos
   passed to memcpy(&buffer[old_pos], ...) — a potential out-of-bounds write.
  
  Issues / Suggestions
  
  1. Enum ordering matters: The new enum values STMT_INVALID_STRING_OR_BUFFER_LENGTH and
  STMT_ATTEMPT_TO_CONCATENATE_NULL_VALUE are inserted between STMT_INVALID_ARGUMENT_NO and STMT_ROW_OUT_OF_RANGE.
  Since the enum is used as an index into the Statement_sqlstate[] array in statement.c, the array entry order must
  match exactly. The PR does this correctly — both the enum and the array are updated in the same relative position.
  ✅
  2. SQL_DEFAULT_PARAM on first call: The code now allows SQL_DEFAULT_PARAM through to putlen = cbValue, and then
  *current_pdata->EXEC_used = putlen stores it. But the existing early-exit for SQL_NULL_DATA (if (cbValue ==
  SQL_NULL_DATA) { retval = SQL_SUCCESS; goto cleanup; }) doesn't have a corresponding early-exit for
  SQL_DEFAULT_PARAM. This means SQL_DEFAULT_PARAM (-2) will fall through to the malloc(putlen + 1) path with a
  negative size. This is a pre-existing bug, not introduced by this PR, but worth noting — the PR could add a similar
  early-exit for SQL_DEFAULT_PARAM or document why it's left as-is.
  3. Second-call check placement: The new guard if (*current_pdata->EXEC_used < 0) is placed before old_pos =
  *current_pdata->EXEC_used in the non-LO else branch. However, the LO (large object) branch above it does
  *current_pdata->EXEC_used += putlen without the same check. If someone calls SQLPutData with SQL_NULL_DATA on a
  large-object parameter and then calls again, the LO path would still proceed. This is likely fine in practice (the
  first call would have failed to open the LO fd), but for consistency the check could be placed before the LO/non-LO
  branch split.
  4. Test doesn't exercise HY020 path: The test only covers the HY090 (invalid length) path. A second test case that
  first sends SQL_NULL_DATA and then attempts a second SQLPutData with valid data would exercise the HY020 path. Not a
  blocker, but would improve coverage.
  5. Minor style: The error message strings are clear and descriptive. The func variable is already in scope for the
  error context. No issues here.

view thread (3+ messages)  latest in thread

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 #182: Reject invalid SQLPutData lengths
  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