public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
2+ messages / 2 participants
[nested] [flat]
* [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
@ 2026-03-24 19:57 Thiago Caserta <[email protected]>
2026-03-26 23:54 ` Re: [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo David G. Johnston <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Thiago Caserta @ 2026-03-24 19:57 UTC (permalink / raw)
To: pgsql-hackers
Hi hackers,
Attached is a patch that converts several sprintf() calls to snprintf() in libpq client library code. While the existing buffers are currently sized correctly, using snprintf() provides an additional safety net against potential buffer overflows and is consistent with the project's general direction of preferring bounded string operations.
Changes:
- fe-auth.c: SSPI target string construction
- fe-connect.c: client encoding query formatting
- fe-exec.c: notice message formatting
- fe-print.c: format string construction
- win32.c: Windows socket error messages
The patch applies cleanly against current HEAD (dd5716f3c74) and passes git diff --check with no whitespace issues. No functional changes are introduced (this is a safety hardening change only).
Best regards,
Thiago Caserta
Attachments:
[application/octet-stream] v1-0001-Replace-sprintf-with-snprintf-in-libpq-for-safety.patch (3.9K, 3-v1-0001-Replace-sprintf-with-snprintf-in-libpq-for-safety.patch)
download | inline diff:
From e8dc7d79ca6277177918b36b368aa1ee4a01d1ed Mon Sep 17 00:00:00 2001
From: Thiago Caserta <[email protected]>
Date: Tue, 24 Mar 2026 16:32:39 -0300
Subject: [PATCH v1] Replace sprintf() with snprintf() in libpq for safety
Convert several sprintf() calls to snprintf() in libpq client
library code to prevent potential buffer overflows. While most
of these buffers are currently sized correctly, using snprintf()
is a safer practice that guards against future changes.
Affected files:
- fe-auth.c: SSPI target string construction
- fe-connect.c: client encoding query formatting
- fe-exec.c: notice message formatting
- fe-print.c: format string construction
- win32.c: Windows socket error messages
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---
src/interfaces/libpq/fe-auth.c | 3 ++-
src/interfaces/libpq/fe-connect.c | 2 +-
src/interfaces/libpq/fe-exec.c | 2 +-
src/interfaces/libpq/fe-print.c | 4 ++--
src/interfaces/libpq/win32.c | 4 ++--
5 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index f05aaea9651..a40a4bcffa4 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -416,7 +416,8 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate, int payloadlen)
libpq_append_conn_error(conn, "out of memory");
return STATUS_ERROR;
}
- sprintf(conn->sspitarget, "%s/%s", conn->krbsrvname, host);
+ snprintf(conn->sspitarget, strlen(conn->krbsrvname) + strlen(host) + 2,
+ "%s/%s", conn->krbsrvname, host);
/*
* Indicate that we're in SSPI authentication mode to make sure that
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index db9b4c8edbf..1e35bebafda 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -7854,7 +7854,7 @@ PQsetClientEncoding(PGconn *conn, const char *encoding)
return -1;
/* ok, now send a query */
- sprintf(qbuf, query, encoding);
+ snprintf(qbuf, sizeof(qbuf), query, encoding);
res = PQexec(conn, qbuf);
if (res == NULL)
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 203d388bdbf..32baabfb787 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -976,7 +976,7 @@ pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt,...)
*/
res->errMsg = (char *) pqResultAlloc(res, strlen(msgBuf) + 2, false);
if (res->errMsg)
- sprintf(res->errMsg, "%s\n", msgBuf);
+ snprintf(res->errMsg, strlen(msgBuf) + 2, "%s\n", msgBuf);
else
res->errMsg = libpq_gettext("out of memory\n");
diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index c4c709bc3e4..593bf6a434d 100644
--- a/src/interfaces/libpq/fe-print.c
+++ b/src/interfaces/libpq/fe-print.c
@@ -717,9 +717,9 @@ PQprintTuples(const PGresult *res,
nTups = PQntuples(res);
if (colWidth > 0)
- sprintf(formatString, "%%s %%-%ds", colWidth);
+ snprintf(formatString, sizeof(formatString), "%%s %%-%ds", colWidth);
else
- sprintf(formatString, "%%s %%s");
+ snprintf(formatString, sizeof(formatString), "%%s %%s");
if (nFields > 0)
{ /* only print rows with at least 1 field. */
diff --git a/src/interfaces/libpq/win32.c b/src/interfaces/libpq/win32.c
index b0c558b55a5..5d28f5999fe 100644
--- a/src/interfaces/libpq/win32.c
+++ b/src/interfaces/libpq/win32.c
@@ -307,14 +307,14 @@ winsock_strerror(int err, char *strerrbuf, size_t buflen)
}
if (!success)
- sprintf(strerrbuf, libpq_gettext("unrecognized socket error: 0x%08X/%d"), err, err);
+ snprintf(strerrbuf, buflen, libpq_gettext("unrecognized socket error: 0x%08X/%d"), err, err);
else
{
strerrbuf[buflen - 1] = '\0';
offs = strlen(strerrbuf);
if (offs > (int) buflen - 64)
offs = buflen - 64;
- sprintf(strerrbuf + offs, " (0x%08X/%d)", err, err);
+ snprintf(strerrbuf + offs, buflen - offs, " (0x%08X/%d)", err, err);
}
return strerrbuf;
}
--
2.53.0
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
2026-03-24 19:57 [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo Thiago Caserta <[email protected]>
@ 2026-03-26 23:54 ` David G. Johnston <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: David G. Johnston @ 2026-03-26 23:54 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: Thiago Caserta <[email protected]>; pgsql-hackers
On Thu, Mar 26, 2026 at 4:33 PM Álvaro Herrera <[email protected]> wrote:
> On 2026-Mar-24, Thiago Caserta wrote:
>
> > Attached is a patch that converts several sprintf() calls to
> > snprintf() in libpq client library code.
>
> I'm not sure we should take a patch with a tag attributing authorship to
> an LLM owned by a commercial entity.
Agreed. As with a book author, any bad code, decisions, or other mistakes
are solely the fault of the submitting author. As is the good stuff.
Ideally the author has confirmed it is good (in their own opinion) since
they expect others to do so as well as part of the review and commit
process.
It is in fact a reputational thing for authors to take full ownership of
what they submit.
> Do we really want to be accepting code patches written by tools that
> make the most obvious code worse than before? I am quite scared of the
> quality of code of medium complexity written by this tool.
>
>
I'd say take this as an opportunity to teach (or not) just as if the author
of patch claimed to write the entire thing without AI assistance. But it
would definitely be reasonable for a hastily produced patch that doesn't
pass muster to be hastily rejected on such grounds. We have plenty to
review and if this patch wouldn't have existed without LLM assistance then
unless it sparks the interest in someone to go and clean it up anyway we
are not really any worse off being quick to state that it doesn't meet our
standards.
Otherwise, while there is a patch, maybe just treat it as a simple
suggestion with an example.
David J.
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-03-26 23:54 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-24 19:57 [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo Thiago Caserta <[email protected]>
2026-03-26 23:54 ` David G. Johnston <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox