Message-ID: <gh-postgresql-interfaces-psqlodbc-176@github.com>
From: "jarvis24young (@jarvis24young)" <noreply+jarvis24young@github.com>
To: "postgresql-interfaces/psqlodbc" <noreply+postgresql-interfaces-psqlodbc@github.com>
Date: Fri, 24 Apr 2026 09:15:48 +0000
Subject: [postgresql-interfaces/psqlodbc] PR #176: Avoid reading past conn_settings end
List-Id: <gh-postgresql-interfaces-psqlodbc.github.com>
X-GitHub-Author-Id: 48787405
X-GitHub-Author-Login: jarvis24young
X-GitHub-Issue: 176
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/176
Content-Type: text/plain; charset=utf-8

## Summary

`check_client_encoding()` can read one byte past the end of `conn_settings` when the `client_encoding` assignment is the final command in the string and is not followed by a semicolon.

This patch stops the parser once it has reached the terminating NUL byte, and adds a regression test for a terminal `ConnSettings=set+client_encoding+to+UTF8` value.

## Problem

The parser walks `conn_settings` with a `for` loop whose iteration expression increments `cptr` after each state-machine step. In the value-parsing state, when the value is the last token in the string, the inner scan leaves `cptr` positioned on the string terminator.

Without an explicit stop at that point, the outer `for` loop increments `cptr` once more and the next loop condition evaluates `*cptr` past the end of the allocated string. AddressSanitizer reports this as a heap buffer over-read for connection strings where `client_encoding` appears at the end of `ConnSettings`.

## ASan reproduction before the fix

Using the new regression test against the pre-fix `multibyte.c` (`HEAD^`) in my WSL ASan/UBSan build tree:

`ConnSettings=set+client_encoding+to+UTF8`

The focused suite fails with:

```text
not ok 1 - conn-settings test returned 256
ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1
#0 check_client_encoding /home/yjw/psqlodbc-build/multibyte.c:129
#1 CC_initial_log /home/yjw/psqlodbc-build/connection.c:1053
#2 LIBPQ_CC_connect /home/yjw/psqlodbc-build/connection.c:1092
#3 CC_connect /home/yjw/psqlodbc-build/connection.c:1142
#4 PGAPI_DriverConnect /home/yjw/psqlodbc-build/drvconn.c:233
#5 SQLDriverConnect /home/yjw/psqlodbc-build/odbcapi.c:213
#6 test_connect_ext src/common.c:92
#7 main src/conn-settings-test.c:12
```

ASan also identifies the offending address as immediately after the decoded `conn_settings` allocation:

```text
0x503000003a4c is located 0 bytes after 28-byte region [0x503000003a30,0x503000003a4c)
allocated by thread T0 here:
#0 strdup
#1 decode /home/yjw/psqlodbc-build/dlg_specific.c:1635
#2 decode_or_remove_braces /home/yjw/psqlodbc-build/dlg_specific.c:1673
#3 copyConnAttributes /home/yjw/psqlodbc-build/dlg_specific.c:686
#4 dconn_get_attributes /home/yjw/psqlodbc-build/drvconn.c:577
```

## Fix

After each state-machine step, the parser now checks whether `cptr` is on `\0` and breaks out before the `for` loop can advance it past the terminator.

This keeps the existing behavior for semicolon-separated commands intact, including the existing logic that backs up over `;` so the outer loop can process command boundaries consistently.

## Test

Added `conn-settings-test`, which connects with:

`ConnSettings=set+client_encoding+to+UTF8`

This exercises the previously problematic terminal `client_encoding` value without a trailing semicolon.

## Local verification after the fix

- Rebuilt the WSL psqlODBC build tree with `make -j4` under the existing ASan/UBSan/gcov configuration.
- Built the new test binary with `make LIBODBC='-lodbc' exe/conn-settings-test`.
- Ran `./runsuite conn-settings --inputdir=.` and got `ok 1 - conn-settings`.