On Thu, Sep 19, 2024 at 12:44:32PM -0500, Nathan Bossart wrote:
> On Thu, Sep 19, 2024 at 10:31:15AM -0400, Tom Lane wrote:
>> We could put an arbitrary limit (say, half of BLCKSZ) on the length of
>> passwords.
>
> Something like that could be good enough. I was thinking about actually
> validating that the hash had the correct form, but that might be a little
> more complex than is warranted here.
Oh, actually, I see that we are already validating the hash, but you can
create valid SCRAM-SHA-256 hashes that are really long. So putting an
arbitrary limit (patch attached) is probably the correct path forward. I'd
also remove pg_authid's TOAST table while at it.
--
nathan
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 629e51e00b..a723e8219a 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -121,6 +121,19 @@ encrypt_password(PasswordType target_type, const char
*role,
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
{
+ /*
+ * Valid SCRAM-SHA-256 hashes can have very long "iterations"
and
+ * "salt" fields, but we don't want to store anything that
might get
+ * TOASTed, since de-TOASTing won't work during authentication
because
+ * we haven't selected a database yet and cannot read pg_class.
256
+ * bytes should be more than enough for all practical use, so
fail for
+ * anything longer.
+ */
+ if (guessed_type == PASSWORD_TYPE_SCRAM_SHA_256 &&
+ strlen(password) > 256)
+ ereport(ERROR,
+ (errmsg("SCRAM-SHA-256 hash is too
long")));
+
/*
* Cannot convert an already-encrypted password from one format
to
* another, so return it as it is.
