Am Sonntag, dem 06.04.2025 um 15:43 -0400 schrieb Tom Lane: > I'd be more comfortable with a check like > > if (strchr("...valid chars...", *ep) != NULL) > > It looks like "_crypt_itoa64" might be directly usable as the > valid-chars string, too. (BTW, why is _crypt_itoa64 not > marked const?)
Here is a patch that tries to address all these issues (including Andres' report). I've adjusted the error message and use ereport(), so it might be more useful if we deal with not just single byte letters. I've also changed _crypt_itoa64 from unsigned char to char, since this seems what strchr() expects (at least on my machine) and we don't deal specifically elsewhere with that. Thanks, Bernd
From d9aa1524a8280bb3997998c2a3499b1f8cfdee6c Mon Sep 17 00:00:00 2001 From: Bernd Helmle <Bernd Helmle maili...@oopsware.de> Date: Mon, 7 Apr 2025 13:45:12 +0200 Subject: [PATCH] Follow up fixes for commit 749a9e20c9790006f3af47f7a8faf4ad8dc358d9. This removes the platform dependant use of isalpha() and friends and replaces the check for supported letters in the salt string to just check against plain ASCII letters. While this makes the use of salt letters more strict, we expect this to make the code more robust. This fixes a careless use-pointer-after-pfree spotted by Andres Freund, too. --- contrib/pgcrypto/crypt-sha.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c index 7e9440ca784..bcf5b480732 100644 --- a/contrib/pgcrypto/crypt-sha.c +++ b/contrib/pgcrypto/crypt-sha.c @@ -58,7 +58,7 @@ typedef enum PGCRYPTO_SHA_UNKOWN } PGCRYPTO_SHA_t; -static unsigned char _crypt_itoa64[64 + 1] = +static const char _crypt_itoa64[64 + 1] = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; /* @@ -321,10 +321,12 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle if (*ep != '$') { - if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/')) + if (strchr(_crypt_itoa64, *ep) != NULL) appendStringInfoCharMacro(decoded_salt, *ep); else - elog(ERROR, "invalid character in salt string: \"%c\"", *ep); + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid character in salt string at: \"%s\"", ep)); } else { @@ -602,8 +604,6 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle elog(ERROR, "unsupported digest length"); } - *cp = '\0'; - /* * Copy over result to specified buffer. * -- 2.49.0