Decision by Monday: PQescapeString() vs. encoding violation

From: Noah Misch <noah(at)leadboat(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: andres(at)anarazel(dot)de, pgsql(at)j-davis(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us
Subject: Decision by Monday: PQescapeString() vs. encoding violation
Date: 2025-02-15 01:27:12
Message-ID: 20250215012712.45@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The security team has a question below about how best to proceed with a recent
behavior change.

Commit 5dc1e42b4fa6a4434afa7d7cdcf0291351a7b873 for this week's CVE-2025-1094
changed how PQescapeString()[1] reacts to input that is not valid in the
client encoding. Before that commit, the function would ignore encoding
problems except at the end of the string. Now, it replaces the bad sequence
up to the length implied by the first byte. For example, if UTF8 input has
0xc2 followed by an ASCII byte, the function removes both bytes.

Jeff Davis reported to the security team that
http://www.unicode.org/reports/tr36/#Ill-Formed_Subsequences forbids something
like this, saying a UTF-8 converter "must not consume the [second byte] if it
continues". (That's my summary, not his. He might reply to add color here.)
While PQescapeString() is not a UTF-8 converter, that standard still felt to
multiple security team members like a decent argument for removing only the
invalid 0xc2, not the byte following it. UTF8 is the most important encoding,
and other encodings tolerate this. Either way, the server will report an
encoding error. The difference doesn't have functional consequences if one
simply puts the function result in a query. The difference could matter for
debugging or if applications are postprocessing the PQescapeString() result in
some way. Postprocessing is not supported, but we'd still like to do the best
thing for applications that may already be doing it.

Security team members disagreed on whether next week's releases are the last
reasonable chance to change this, or whether changing it in e.g. May would be
reasonable. If applications make changes to cope with the new behavior, that
could be an argument against further change.

Question for all: would you switch to the "remove fewer bytes" behavior in
next week's releases, switch later, or switch never? Why so? Please answer
in the next 24hr if possible, given the little time until we wrap next week's
releases on Monday. I regret the late notice.

I'm attaching a WIP patch from Andres Freund. We may use it to adopt the
"remove fewer bytes" behavior, if that's the decision.

Thanks,
nm

[1] The commit changed other functions, but PQescapeString() is most
interesting for this discussion. Others have ways to report errors, or they
have reason to believe the input is already checked. New code should be using
the others and checking the error indicator.

Attachment Content-Type Size
v3-0002-Have-escape-functions-process-bytes-after-invalid.patch text/plain 5.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2025-02-15 01:34:59 Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible
Previous Message Thomas Munro 2025-02-15 01:23:08 Re: Confine vacuum skip logic to lazy_scan_skip