Re: BUG #19600: pgcrypto crypt() can return text with invalid encoding

On Mon, Aug 3, 2026 at 15:26, Andrey Rachitskiy wrote:
> The attached patch adds pg_verifymbstr() in pg_crypt().

Hi Andrey,

I reviewed and tested the patch on master (700b422af7e), built with
--with-ssl=openssl.

Without the patch, in a UTF8 database, I could reproduce the DES case
from the report, and also the xDES and MD5 cases it describes: a salt
whose multibyte character straddles the copied prefix ("€A" for DES,
"_J9..ab€" for xDES, "$1$1234567€" for MD5) makes crypt() return
invalid UTF-8, and a table holding the DES result can't be reloaded
with COPY. DES also returns invalid text in an EUC_JP database (salt
"A" followed by a two-byte character). SHA-crypt and Blowfish already
reject non-ASCII salts.

With the patch, each of those cut-off cases raises "invalid byte
sequence for encoding", and the ASCII and valid multibyte salts I tried
return the same hashes as before, which matches the code: the new check
only reads the result after it has been built. The pgcrypto tests pass
with UTF8, LATIN1 and SQL_ASCII databases (crypt-utf8 runs in the first
and is skipped in the others), and crypt-utf8 fails without the code
change.

Two points:

1. Hashes already stored with invalid encoding stop working after the
upgrade: crypt(pw, stored_hash) now raises an error instead of
returning a hash to compare, for the right password as well as a
wrong one. That seems acceptable to me, since the stored value
already can't be dumped and restored, and it's a similar trade-off
to the one the PGP decrypt functions made in d536aee5566, although
unlike those, crypt() has no bytea variant to fall back on. It may
be worth a mention in the release notes.

2. The new test covers only DES. Since the fix handles xDES and MD5 as
well, it might be worth adding those cases from the report.

For the back branches: the patch applies as-is to REL_18_STABLE and
REL_19_STABLE. On REL_14 to REL_17 the pgcrypto.c change and the new
test files apply cleanly; only the REGRESS list in the Makefile needs
adjusting, and REL_14/REL_15 have no meson.build. These were
applicability checks only; I didn't build the back branches.

Otherwise this looks good to me.

Thanks,
Ishjaap Singh