Thread: pgsql: Make escaping functions retain trailing bytes of an invalid char
Make escaping functions retain trailing bytes of an invalid character. Instead of dropping the trailing byte(s) of an invalid or incomplete multibyte character, replace only the first byte with a known-invalid sequence, and process the rest normally. This seems less likely to confuse incautious callers than the behavior adopted in 5dc1e42b4. While we're at it, adjust PQescapeStringInternal to produce at most one bleat about invalid multibyte characters per string. This matches the behavior of PQescapeInternal, and avoids the risk of producing tons of repetitive junk if a long string is simply given in the wrong encoding. This is a followup to the fixes for CVE-2025-1094, and should be included if cherry-picking those fixes. Author: Andres Freund <andres@anarazel.de> Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/20250215012712.45@rfd.leadboat.com Backpatch-through: 13 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/e782a63ccb76a8db476abb8f5d96397806e3e51b Modified Files -------------- src/fe_utils/string_utils.c | 91 ++++++++++++++++-------------------------- src/interfaces/libpq/fe-exec.c | 73 +++++++++++++++------------------ 2 files changed, 67 insertions(+), 97 deletions(-)
Re: pgsql: Make escaping functions retain trailing bytes of an invalid char
From
Peter Eisentraut
Date:
On 15.02.25 22:20, Tom Lane wrote: > Make escaping functions retain trailing bytes of an invalid character. > > Instead of dropping the trailing byte(s) of an invalid or incomplete > multibyte character, replace only the first byte with a known-invalid > sequence, and process the rest normally. This seems less likely to > confuse incautious callers than the behavior adopted in 5dc1e42b4. > > While we're at it, adjust PQescapeStringInternal to produce at most > one bleat about invalid multibyte characters per string. This > matches the behavior of PQescapeInternal, and avoids the risk of > producing tons of repetitive junk if a long string is simply given > in the wrong encoding. > > This is a followup to the fixes for CVE-2025-1094, and should be > included if cherry-picking those fixes. > > Author: Andres Freund <andres@anarazel.de> > Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> > Reported-by: Jeff Davis <pgsql@j-davis.com> > Discussion: https://postgr.es/m/20250215012712.45@rfd.leadboat.com > Backpatch-through: 13 > > Branch > ------ > REL_15_STABLE > > Details > ------- > https://git.postgresql.org/pg/commitdiff/e782a63ccb76a8db476abb8f5d96397806e3e51b > > Modified Files > -------------- > src/fe_utils/string_utils.c | 91 ++++++++++++++++-------------------------- > src/interfaces/libpq/fe-exec.c | 73 +++++++++++++++------------------ > 2 files changed, 67 insertions(+), 97 deletions(-) I think the backpatch of this to PG15 and older has erroneously dropped newlines at the end of some error messages. (libpq in newer PG uses libpq_append_conn_error() etc. and does not have newlines in error message strings anymore.) I think this fix would be appropriate for PG15 and older: diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 4a4731d048c..73c3a0f9025 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -4040,10 +4040,10 @@ PQescapeStringInternal(PGconn *conn, { if (remaining < charlen) appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("incomplete multibyte character")); + libpq_gettext("incomplete multibyte character\n")); else appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("invalid multibyte character")); + libpq_gettext("invalid multibyte character\n")); /* Issue a complaint only once per string */ already_complained = true; }
Peter Eisentraut <peter@eisentraut.org> writes:
> On 15.02.25 22:20, Tom Lane wrote:
>> Make escaping functions retain trailing bytes of an invalid character.
> I think the backpatch of this to PG15 and older has erroneously dropped
> newlines at the end of some error messages.
Oh! You're right, though I'd phrase it differently: I should have
*added* newlines when back-patching into those branches. Will fix.
regards, tom lane