From 8dbf7862942aa6414aa47e7642cd52cd72127964 Mon Sep 17 00:00:00 2001 From: Prashant Chinnam Date: Tue, 27 Oct 2026 18:13:51 +0000 Subject: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743) px_crypt_shacrypt() parsed the rounds= option of a sha256crypt or sha512crypt salt string with strtoint(), which historically signaled a value that does not fit into an int only by setting errno to ERANGE, while returning the value cast to int. 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". strtoint() now saturates to INT_MAX or INT_MIN on overflow (commit 426551c8cf49, backpatched through PG 14), so an out-of-range value lands directly in the existing supported-range clamping, and this caller no longer needs to check errno or parse into a wider type. This patch can therefore be backpatched along with that change. Have the clamp NOTICEs report the value as supplied instead of the converted value, so that a saturated conversion result is never reported either. The supplied token is part of the caller-provided salt string, whose length is not bounded, so cap the value echoed by the clamp NOTICEs at 10 characters -- one more than the number of digits of the maximum supported value, from which the limit is derived -- and append "..." when it was truncated. Only the NOTICE text changes; the parsing and clamping behavior are unaffected. Add regression tests for negative and positive overflow of the rounds= value, and for the truncated echo of an overlong token. 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 overlong token is built with repeat() rather than written out as a literal. Earlier versions of this patch parsed with strtoi64(), an approach suggested by Takeshi Ideriha; the final version instead relies on strtoint() saturating on overflow. Capping the printed rounds value, deriving the limit from the maximum rounds value, and building the overlong test token with repeat() were suggested by Chao Li, author of the strtoint() change. Bug: #19743 Reported-by: Ke Discussion: https://postgr.es/m/19743-c48782d257b89819@postgresql.org --- contrib/pgcrypto/crypt-sha.c | 20 ++++-- contrib/pgcrypto/expected/crypt-shacrypt.out | 69 ++++++++++++++++++++ contrib/pgcrypto/px-crypt.h | 11 ++++ contrib/pgcrypto/sql/crypt-shacrypt.sql | 39 +++++++++++ 4 files changed, 135 insertions(+), 4 deletions(-) diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c index dfdea1f..3521c92 100644 --- a/contrib/pgcrypto/crypt-sha.c +++ b/contrib/pgcrypto/crypt-sha.c @@ -187,6 +187,8 @@ 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); + int numlen = (int) (endp - num); + bool num_truncated = numlen > PX_SHACRYPT_ROUNDS_PRINT_MAX; if (*endp != '$') ereport(ERROR, @@ -195,6 +197,14 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle dec_salt_binary = endp + 1; + /* + * Limit how much of the supplied rounds value the NOTICEs below echo + * back; the token is part of the caller-supplied salt string and can + * be arbitrarily long. + */ + if (num_truncated) + numlen = PX_SHACRYPT_ROUNDS_PRINT_MAX; + /* * We violate supported lower or upper bound of rounds, but in this * case we change this value to the supported lower or upper value. We @@ -210,8 +220,9 @@ 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%s exceeds maximum supported value (%d), using %d instead", + numlen, num, num_truncated ? "..." : "", + PX_SHACRYPT_ROUNDS_MAX, PX_SHACRYPT_ROUNDS_MAX)); srounds = PX_SHACRYPT_ROUNDS_MAX; } @@ -219,8 +230,9 @@ 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%s is below supported value (%d), using %d instead", + numlen, num, num_truncated ? "..." : "", + 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..48c1696 100644 --- a/contrib/pgcrypto/expected/crypt-shacrypt.out +++ b/contrib/pgcrypto/expected/crypt-shacrypt.out @@ -192,5 +192,74 @@ 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=-429496829... 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=-429496829... 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=-999999999... 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=-429496229... 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=9999999999... exceeds maximum supported value (999999999), using 999999999 instead +ERROR: canceling statement due to statement timeout +RESET statement_timeout; +-- A rounds token longer than PX_SHACRYPT_ROUNDS_PRINT_MAX is echoed +-- truncated, with an ellipsis, in the clamp NOTICE; the token below +-- is 9999 zeros followed by a 5, so it clamps to the minimum like +-- rounds=5 does. +SELECT crypt('password', '$5$rounds=' || repeat('0', 9999) || '5$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; +NOTICE: rounds=0000000000... is below supported value (1000), using 1000 instead + result +-------- + t +(1 row) + -- cleanup DROP TABLE ctest; diff --git a/contrib/pgcrypto/px-crypt.h b/contrib/pgcrypto/px-crypt.h index bb03cf5..f341007 100644 --- a/contrib/pgcrypto/px-crypt.h +++ b/contrib/pgcrypto/px-crypt.h @@ -69,6 +69,17 @@ /* Maximum number of rounds of shacrypt. */ #define PX_SHACRYPT_ROUNDS_MAX 999999999 +/* + * Maximum length of the rounds value printed in the clamp NOTICEs of + * px_crypt_shacrypt(). The supplied token is echoed so that an + * out-of-range value is reported as given, but it comes from the + * caller-supplied salt string and can be arbitrarily long; cap the + * echo at one more than the number of digits of the maximum + * supported value, and mark where the value was truncated. + */ +#define PX_SHACRYPT_ROUNDS_PRINT_MAX \ + ((int) sizeof(CppAsString2(PX_SHACRYPT_ROUNDS_MAX))) + /* * main interface */ diff --git a/contrib/pgcrypto/sql/crypt-shacrypt.sql b/contrib/pgcrypto/sql/crypt-shacrypt.sql index 3ee826f..b034537 100644 --- a/contrib/pgcrypto/sql/crypt-shacrypt.sql +++ b/contrib/pgcrypto/sql/crypt-shacrypt.sql @@ -95,5 +95,44 @@ 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; + +-- A rounds token longer than PX_SHACRYPT_ROUNDS_PRINT_MAX is echoed +-- truncated, with an ellipsis, in the clamp NOTICE; the token below +-- is 9999 zeros followed by a 5, so it clamps to the minimum like +-- rounds=5 does. +SELECT crypt('password', '$5$rounds=' || repeat('0', 9999) || '5$saltsalt$') + = '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result; + -- cleanup DROP TABLE ctest; -- 2.43.0