From 5db105888cfe49527238183903c859179701e6b5 Mon Sep 17 00:00:00 2001 From: OrbisAI Security Date: Mon, 25 May 2026 15:18:08 +0530 Subject: [PATCH v1] Defensive hardening: Replace sprintf with snprintf in pg_stat_statements Replace unbounded sprintf with snprintf when generating normalized query parameter placeholders ($1, $2, etc.) in pg_stat_statements. This makes the write bound explicit at the formatting site rather than relying on buffer sizing logic elsewhere in the function. While the current buffer sizing rule (query_len + clocations_count * 10) appears to account for worst-case growth, using snprintf provides local verification of safety and protects against potential future modifications to the sizing logic. Additionally, improve NULL pointer safety for qbuffer: - Initialize qbuffer to NULL in gc_qtexts() - Set qbuffer to NULL after pfree() in both gc_qtexts() and pgss_shmem_shutdown() error paths These changes follow defensive programming practices and guard against potential use-after-free scenarios. This is defensive hardening only - no demonstrated overflow vulnerability is being fixed. No performance impact expected. --- contrib/pg_stat_statements/pg_stat_statements.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 9231562..8ea847d 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -820,7 +820,10 @@ error: errmsg("could not write file \"%s\": %m", PGSS_DUMP_FILE ".tmp"))); if (qbuffer) + { pfree(qbuffer); + qbuffer = NULL; + } if (file) FreeFile(file); unlink(PGSS_DUMP_FILE ".tmp"); @@ -2475,7 +2478,7 @@ need_gc_qtexts(void) static void gc_qtexts(void) { - char *qbuffer; + char *qbuffer = NULL; Size qbuffer_size; FILE *qfile = NULL; HASH_SEQ_STATUS hash_seq; @@ -2590,6 +2593,7 @@ gc_qtexts(void) pgss->mean_query_len = ASSUMED_LENGTH_INIT; pfree(qbuffer); + qbuffer = NULL; /* * OK, count a garbage collection cycle. (Note: even though we have @@ -2607,7 +2611,10 @@ gc_fail: if (qfile) FreeFile(qfile); if (qbuffer) + { pfree(qbuffer); + qbuffer = NULL; + } /* * Since the contents of the external file are now uncertain, mark all @@ -2880,9 +2887,11 @@ generate_normalized_query(const JumbleState *jstate, const char *query, * we have a squashable list, insert a placeholder comment starting * from the list's second value. */ - n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d%s", - num_constants_replaced + 1 + jstate->highest_extern_param_id, - locs[i].squashed ? " /*, ... */" : ""); + n_quer_loc += snprintf(norm_query + n_quer_loc, + norm_query_buflen - n_quer_loc + 1, + "$%d%s", + num_constants_replaced + 1 + jstate->highest_extern_param_id, + locs[i].squashed ? " /*, ... */" : ""); num_constants_replaced++; /* move forward */ -- 2.39.5 (Apple Git-154)