Skip to content

fix: hoist intToString() temporaries to avoid dangling strValue pointers - #1133

Merged
dimitri merged 1 commit into
mainfrom
fix/intstring-dangling-pointer
Jul 9, 2026
Merged

fix: hoist intToString() temporaries to avoid dangling strValue pointers#1133
dimitri merged 1 commit into
mainfrom
fix/intstring-dangling-pointer

Conversation

@dimitri

@dimitri dimitri commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The bug

intToString() returns an IntString struct by value. Taking .strValue from the result inline:

paramValues[1] = intToString(port).strValue;

stores a pointer into a temporary struct that is destroyed at the end of the statement. Any subsequent read of paramValues[1] — including the pgsql_execute_with_params() call a few lines later — reads freed stack memory. This is undefined behaviour; in practice it produces garbage bytes in libpq parameter arrays, which propagate into PostgreSQL error messages and can cause UnicodeDecodeError in any Python code that processes those messages.

Clang catches this with -Wdangling-assignment.

The fix

Declare a named IntString variable at function scope so that .strValue remains valid across the entire parameter-building and query-execution block:

IntString portStr = intToString(port);
paramValues[1] = portStr.strValue;
/* ... fill remaining paramValues ... */
pgsql_execute_with_params(..., paramValues, ...);   /* safe: portStr still in scope */

Files changed

File Instances fixed
src/bin/pg_autoctl/monitor.c 20 — across 13 functions
src/bin/pg_autoctl/coordinator.c 10 — across 5 functions
src/bin/pg_autoctl/demoapp.c 8 — across 3 functions
src/bin/pg_autoctl/pgctl.c 1 — pg_ctl_postgres args array
src/bin/pg_autoctl/pgsql.c 1 — send_node_active_message loop
src/bin/pg_autoctl/ipaddr.c 1 — fetchLocalIPAddress GetAddrInfo call

41 instances total. No logic changes; only the lifetime of the intermediate struct is extended.

citus_indent --check passes. -Wdangling-assignment is clean after this change.

intToString() returns an IntString struct by value.  Taking .strValue
from a compound literal like intToString(x).strValue produces a pointer
into a struct that is destroyed at the end of the statement.  Any use
of that pointer after the semicolon is undefined behaviour; when the
value is stored in a paramValues[] array and read by a subsequent
pgsql_execute_with_params() call, it reliably reads freed stack memory.

Fix: declare a named IntString variable at function scope so that
.strValue remains valid across the entire parameter-building and
query-execution block.

Files and instance counts:
  monitor.c     20 instances across 13 functions
  coordinator.c 10 instances across 5 functions
  demoapp.c      8 instances across 3 functions
  pgctl.c        1 instance  (pg_ctl_postgres args array)
  pgsql.c        1 instance  (send_node_active_message loop)
  ipaddr.c       1 instance  (fetchLocalIPAddress GetAddrInfo call)

Clang -Wdangling-assignment is clean after this change.
@dimitri dimitri self-assigned this Jul 9, 2026
@dimitri dimitri added the bug Something isn't working label Jul 9, 2026
@dimitri
dimitri merged commit 5e91db1 into main Jul 9, 2026
51 of 52 checks passed
@dimitri
dimitri deleted the fix/intstring-dangling-pointer branch July 9, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant