From 716cba59588f98b0a7e1cb48e2f30034b26a7182 Mon Sep 17 00:00:00 2001 From: Prashant Chinnam Date: Tue, 27 Oct 2026 00:41:16 +0000 Subject: [PATCH v2] pgcrypto: Fix sha-crypt rounds= values that do not fit in int px_crypt_shacrypt() parsed the rounds= option of a sha256crypt or sha512crypt salt string with strtoint(), which signals a value that does not fit into an int only by setting errno to ERANGE. The result was never checked, so an out-of-range value was reduced modulo 2^32 before the supported-range clamping ran: rounds=4294968296 silently produced a hash with the minimum of 1000 rounds and no NOTICE at all, and rounds=4294967295 was reported as "rounds=-1". Parse the value with strtoi64() instead. A value that does not fit into an int64 saturates to INT64_MAX or INT64_MIN, and any smaller out-of-range value converts exactly, so every out-of-range value now lands in the existing supported-range clamping. Have the clamp NOTICEs report the value as supplied instead of the converted value, so that a saturated conversion result is never reported either. Add regression tests for negative and positive overflow of the rounds= value. The positive cases are interrupted with statement_timeout after the NOTICE, since actually computing the clamped maximum of 999999999 rounds would take far too long. The strtoi64 parsing approach was suggested by Takeshi Ideriha. Bug: #19743 Reported-by: Ke Discussion: https://postgr.es/m/19743-c48782d257b89819@postgresql.org --- contrib/pgcrypto/crypt-sha.c | 10 ++-- contrib/pgcrypto/expected/crypt-shacrypt.out | 57 ++++++++++++++++++++ contrib/pgcrypto/sql/crypt-shacrypt.sql | 32 +++++++++++ 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c index dfdea1f..60c9c0c 100644 --- a/contrib/pgcrypto/crypt-sha.c +++ b/contrib/pgcrypto/crypt-sha.c @@ -186,7 +186,7 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle { const char *num = dec_salt_binary + sizeof(rounds_prefix) - 1; char *endp; - int srounds = strtoint(num, &endp, 10); + int64 srounds = strtoi64(num, &endp, 10); if (*endp != '$') ereport(ERROR, @@ -210,8 +210,8 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle { ereport(NOTICE, errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("rounds=%d exceeds maximum supported value (%d), using %d instead", - srounds, PX_SHACRYPT_ROUNDS_MAX, + errmsg("rounds=%.*s exceeds maximum supported value (%d), using %d instead", + (int) (endp - num), num, PX_SHACRYPT_ROUNDS_MAX, PX_SHACRYPT_ROUNDS_MAX)); srounds = PX_SHACRYPT_ROUNDS_MAX; } @@ -219,8 +219,8 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle { ereport(NOTICE, errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("rounds=%d is below supported value (%d), using %d instead", - srounds, PX_SHACRYPT_ROUNDS_MIN, + errmsg("rounds=%.*s is below supported value (%d), using %d instead", + (int) (endp - num), num, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MIN)); srounds = PX_SHACRYPT_ROUNDS_MIN; } diff --git a/contrib/pgcrypto/expected/crypt-shacrypt.out b/contrib/pgcrypto/expected/crypt-shacrypt.out index f3b76d5..664e007 100644 --- a/contrib/pgcrypto/expected/crypt-shacrypt.out +++ b/contrib/pgcrypto/expected/crypt-shacrypt.out @@ -192,5 +192,62 @@ NOTICE: rounds=10 is below supported value (1000), using 1000 instead t (1 row) +-- Rounds values in a hand-written salt string that do not fit into an int +-- must not wrap around modulo 2^32: they are out of range and get clamped +-- like any other out-of-range value, with a NOTICE reporting the value as +-- supplied. +-- Negative overflow clamps to the minimum, so this is the same hash as +-- rounds=1000, computed above for 'password' and 'saltsalt' +SELECT crypt('password', '$5$rounds=-4294968296$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; +NOTICE: rounds=-4294968296 is below supported value (1000), using 1000 instead + result +-------- + t +(1 row) + +SELECT crypt('password', '$6$rounds=-4294968296$saltsalt$') + = '$6$rounds=1000$saltsalt$Z/J9iYO1iE9xnr8JPQL57ZWsVRtVjrUv3CiWc/wKWseqXgSqn3HFYJ/Ng7YXa8XlLj.wpdAwHOJJzuGFqBBRa0' AS result; +NOTICE: rounds=-4294968296 is below supported value (1000), using 1000 instead + result +-------- + t +(1 row) + +-- A value beyond the range of long is out of range in the same way; +-- this exercises the ERANGE that strtol() itself reports, rather than +-- the int range check in strtoint() +SELECT crypt('password', '$5$rounds=-99999999999999999999$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; +NOTICE: rounds=-99999999999999999999 is below supported value (1000), using 1000 instead + result +-------- + t +(1 row) + +-- This value wrapped to an in-range 5000 rounds before the fix, so it +-- was computed silently at the wrong number of rounds +SELECT crypt('password', '$5$rounds=-4294962296$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; +NOTICE: rounds=-4294962296 is below supported value (1000), using 1000 instead + result +-------- + t +(1 row) + +-- Positive overflow clamps to the maximum. Actually computing 999999999 +-- rounds would take far too long for a regression test, so interrupt the +-- computation with statement_timeout once the NOTICE has been printed. +SET statement_timeout = '1s'; +SELECT crypt('password', '$5$rounds=4294967295$saltsalt$'); +NOTICE: rounds=4294967295 exceeds maximum supported value (999999999), using 999999999 instead +ERROR: canceling statement due to statement timeout +SELECT crypt('password', '$5$rounds=4294968296$saltsalt$'); +NOTICE: rounds=4294968296 exceeds maximum supported value (999999999), using 999999999 instead +ERROR: canceling statement due to statement timeout +SELECT crypt('password', '$5$rounds=99999999999999999999$saltsalt$'); +NOTICE: rounds=99999999999999999999 exceeds maximum supported value (999999999), using 999999999 instead +ERROR: canceling statement due to statement timeout +RESET statement_timeout; -- cleanup DROP TABLE ctest; diff --git a/contrib/pgcrypto/sql/crypt-shacrypt.sql b/contrib/pgcrypto/sql/crypt-shacrypt.sql index 3ee826f..60a238e 100644 --- a/contrib/pgcrypto/sql/crypt-shacrypt.sql +++ b/contrib/pgcrypto/sql/crypt-shacrypt.sql @@ -95,5 +95,37 @@ SELECT crypt('a short string', '$6$rounds=123456$asaltof16chars..') SELECT crypt('the minimum number is still observed', '$6$rounds=10$roundstoolow') = '$6$rounds=1000$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.' AS result; +-- Rounds values in a hand-written salt string that do not fit into an int +-- must not wrap around modulo 2^32: they are out of range and get clamped +-- like any other out-of-range value, with a NOTICE reporting the value as +-- supplied. + +-- Negative overflow clamps to the minimum, so this is the same hash as +-- rounds=1000, computed above for 'password' and 'saltsalt' +SELECT crypt('password', '$5$rounds=-4294968296$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; +SELECT crypt('password', '$6$rounds=-4294968296$saltsalt$') + = '$6$rounds=1000$saltsalt$Z/J9iYO1iE9xnr8JPQL57ZWsVRtVjrUv3CiWc/wKWseqXgSqn3HFYJ/Ng7YXa8XlLj.wpdAwHOJJzuGFqBBRa0' AS result; + +-- A value beyond the range of long is out of range in the same way; +-- this exercises the ERANGE that strtol() itself reports, rather than +-- the int range check in strtoint() +SELECT crypt('password', '$5$rounds=-99999999999999999999$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; + +-- This value wrapped to an in-range 5000 rounds before the fix, so it +-- was computed silently at the wrong number of rounds +SELECT crypt('password', '$5$rounds=-4294962296$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; + +-- Positive overflow clamps to the maximum. Actually computing 999999999 +-- rounds would take far too long for a regression test, so interrupt the +-- computation with statement_timeout once the NOTICE has been printed. +SET statement_timeout = '1s'; +SELECT crypt('password', '$5$rounds=4294967295$saltsalt$'); +SELECT crypt('password', '$5$rounds=4294968296$saltsalt$'); +SELECT crypt('password', '$5$rounds=99999999999999999999$saltsalt$'); +RESET statement_timeout; + -- cleanup DROP TABLE ctest; -- 2.43.0