Message-ID: <gh-postgresql-interfaces-psqlodbc-145@github.com>
From: "markmaker (@markmaker)" <noreply+markmaker@github.com>
To: "postgresql-interfaces/psqlodbc" <noreply+postgresql-interfaces-psqlodbc@github.com>
Date: Fri, 21 Nov 2025 15:05:52 +0000
Subject: [postgresql-interfaces/psqlodbc] PR #145: SQL_C_BINARY data buffer uses NULL terminator, but realloc() may be missed
List-Id: <gh-postgresql-interfaces-psqlodbc.github.com>
X-GitHub-Author-Id: 9963310
X-GitHub-Author-Login: markmaker
X-GitHub-Issue: 145
X-GitHub-Repo: postgresql-interfaces/psqlodbc
X-GitHub-State: merged
X-GitHub-Type: pull_request
X-GitHub-Url: https://github.com/postgresql-interfaces/psqlodbc/pull/145
Content-Type: text/plain; charset=utf-8

# Problem

We get spurious heap corruption.

# Diagnosis

After a steep learning curve in such matters, we finally found a way to pinpoint it using `valgrind` (the key was using the redzone, so it would not immediately corrupt everything): 

```bash
valgrind --tool=memcheck --leak-check=summary --redzone-size=8 <executable>
```
It outputs these sections (snippets).
**EDIT:** installed Debug symbol, now we can even better pinpoint it:

```
==2666205== Invalid write of size 1
==2666205==    at 0x154AEDA7: UnknownInlinedFun (convert.c:6414)
==2666205==    by 0x154AEDA7: convert_from_pgbinary.isra.0 (convert.c:6235)
==2666205==    by 0x154ABE8E: UnknownInlinedFun (convert.c:1115)
==2666205==    by 0x154ABE8E: UnknownInlinedFun (convert.c:1189)
==2666205==    by 0x154ABE8E: copy_and_convert_field.isra.0 (convert.c:1726)
==2666205==    by 0x154754FA: UnknownInlinedFun (convert.c:810)
==2666205==    by 0x154754FA: SC_fetch (statement.c:1839)
==2666205==    by 0x15469B14: PGAPI_ExtendedFetch (results.c:1902)
==2666205==    by 0x15485922: SQLFetchScroll (odbcapi30.c:245)
==2666205==    by 0x556BF4D: SQLFetchScroll (in /usr/lib/x86_64-linux-gnu/libodbc.so.2.0.0)
...

==2666205==  Address 0x180015b8 is 0 bytes after a block of size 3,576 alloc'd
==2666205==    at 0x4851E10: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2666205==    by 0x154ACDDB: UnknownInlinedFun (convert.c:1071)
==2666205==    by 0x154ACDDB: UnknownInlinedFun (convert.c:1189)
==2666205==    by 0x154ACDDB: copy_and_convert_field.isra.0 (convert.c:1726)
==2666205==    by 0x154754FA: UnknownInlinedFun (convert.c:810)
==2666205==    by 0x154754FA: SC_fetch (statement.c:1839)
==2666205==    by 0x15469B14: PGAPI_ExtendedFetch (results.c:1902)
==2666205==    by 0x15485922: SQLFetchScroll (odbcapi30.c:245)
==2666205==    by 0x556BF4D: SQLFetchScroll (in /usr/lib/x86_64-linux-gnu/libodbc.so.2.0.0)
...
```

Due to the sequence of ODBC calls, and the exact size of the buffer (3576), we knew it was the BYTEA column in our row:

```psql
v_base_pg=# select length(prstntpacked) from v.gusrstns where usrstgusr_userv = 46035;
 length 
--------
   3576
(1 row)
```

Because we use bound buffers that are smaller than that, and we haven't yet come to the point of calling `SQLGetData()`, we know this must be an internal buffer that already knows the size of the BYTEA data. Therefore it couldn't be one of _our_ buffers being too small.

It follows that this looks like the extra NULL terminator byte that so plagues the C world. And sure, the code says:

https://github.com/postgresql-interfaces/psqlodbc/blob/6e99e750163d8cf572d71ea958acc7ef3b561ce5/convert.c#L1058-L1066

# Proposed fix

It seems that immediately after this, the `len_for_wcs_term` is not included in the comparison to assess the need of `realloc()`. If by chance the buffer was already exactly `needbuflen` bytes large, it would not be realloc'd.


https://github.com/postgresql-interfaces/psqlodbc/blob/6e99e750163d8cf572d71ea958acc7ef3b561ce5/convert.c#L1067-L1073

Therefore this PR intends to fix that.

~~We were unable to test this due to time constraints (this is currently impeding our rollout, we have our hands full 🤕!)~~

Help in setting up a psqlodbc compilation and local installation and deployment is welcome.