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

Reply via email to