[PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Started by Prashant Chinnam13 days ago13 messageshackers
Jump to latest
#1Prashant Chinnam
crprashant@hotmail.com

Hi hackers,

Attached is a patch for BUG #19743, reported by Ke Han [1]/messages/by-id/19743-c48782d257b89819@postgresql.org:
pgcrypto's crypt() parses the rounds= option of a sha256crypt /
sha512crypt salt with strtoint() and never checks errno, so a
value that does not fit in an int is silently reduced modulo
2^32 before the supported-range clamping runs. For example,
rounds=4294968296 produces a hash computed with the minimum of
1000 rounds and no NOTICE at all, and rounds=4294967295 is
reported as "rounds=-1".

I reproduced this on current master (20devel) before fixing it.

The patch checks errno around the strtoint() call in
px_crypt_shacrypt(). On ERANGE the value is out of range in
the direction of its sign, so it is clamped to
PX_SHACRYPT_ROUNDS_MIN or PX_SHACRYPT_ROUNDS_MAX, matching the
existing clamp-with-NOTICE behaviour for in-range values, and
the NOTICEs now report the value as supplied instead of the
truncated conversion result.

Regression tests are added to crypt-shacrypt: negative overflow
cases for both $5$ and $6$, checked against full hashes, and
positive overflow cases run under a short statement_timeout so
the tests never actually compute ~1e9 rounds. The pgcrypto
regression suite (25 tests) and the core regression suite
(239 tests) both pass with the patch applied.

This is my first patch submission; feedback is welcome.
Testing was done with AI assistance.

[1]: /messages/by-id/19743-c48782d257b89819@postgresql.org

Thanks,
Prashant Chinnam

Attachments:

t254356_1
v1-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchapplication/octet-stream; name=v1-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchDownload+139-3
#2Takeshi Ideriha
iderihatakeshi@gmail.com
In reply to: Prashant Chinnam (#1)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi Prashant

On 2026/10/23 23:30, Prashant Kumar wrote:

Hi hackers,

Attached is a patch for BUG #19743, reported by Ke Han [1]:
pgcrypto's crypt() parses the rounds= option of a sha256crypt /
sha512crypt salt with strtoint() and never checks errno, so a
value that does not fit in an int is silently reduced modulo
2^32 before the supported-range clamping runs. For example,
rounds=4294968296 produces a hash computed with the minimum of
1000 rounds and no NOTICE at all, and rounds=4294967295 is
reported as "rounds=-1".

I reproduced this on current master (20devel) before fixing it.

The patch checks errno around the strtoint() call in
px_crypt_shacrypt(). On ERANGE the value is out of range in
the direction of its sign, so it is clamped to
PX_SHACRYPT_ROUNDS_MIN or PX_SHACRYPT_ROUNDS_MAX, matching the
existing clamp-with-NOTICE behaviour for in-range values, and
the NOTICEs now report the value as supplied instead of the
truncated conversion result.

Thank you for the patch. In practice, most people would use gen_salt()
to generate the salt. But directly passing the raw salt strings is also
valid, so it needs some fixing.

How about using strtol or strtoi64 instead of strtoint to handle the
rounds value that is out of the integer bounds? I attached my patch
using strtoi64. We can handle the out-of-bound positive and negative
value by taking advantage of current px_crypt_shacrypt logic. We can
omit the check if the round values is positive or negative. I use
strtoi64 so that we can cope with the platform difference. (Windows
treats long type as 4 bytes.) What do you think?

+ rounds_overflow = (errno == ERANGE);

Most of the similar postgresql code is just like "if (errno == ERANGE)"
instead of using another variable.

Regarding errno, I may be wrong but I'm thinking we can omit this check
by using strtoi64 (strtol) since underflow or overflow value will change
to LONG_MIN or LONG_MAX and fall into the current bound check logic.

This is another point. I noticed the reference implementation[*] assumes
round value is only positive since it just uses strtoul though there is
no value check if it's negative or not. We might be OK to turn negative
rounds value to error since negative rounds value does not make any sense.

[*] https://www.akkadia.org/drepper/SHA-crypt.txt

Regards,
Takeshi Ideriha

Attachments:

t254356_2
v1-0001-Fix-clamp-behaviour-when-rounds-is-out-of-bounds-.patchtext/plain; charset=UTF-8; name=v1-0001-Fix-clamp-behaviour-when-rounds-is-out-of-bounds-.patchDownload+5-6
#3Prashant Chinnam
crprashant@hotmail.com
In reply to: Takeshi Ideriha (#2)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi Takeshi,

Thank you for the review and for the alternative patch.

How about using strtol or strtoi64 instead of strtoint to handle
the rounds value that is out of the integer bounds? I attached
my patch using strtoi64.

I built your patch on master (f52d4051) and ran the same battery
I used for the original report against unpatched master, my v1,
and your version. The results are identical to v1 on every case:
values between the int and int64 ranges parse exactly and hit
the existing clamps, and values beyond int64 saturate to
INT64_MAX / INT64_MIN and hit them too. Because your NOTICEs
print the supplied token with %.*s, the saturated value never
leaks into the message either: rounds=99999999999999999999
reports the value as supplied and clamps to the maximum, just
as v1 does. The regression tests from v1 also pass unchanged
against your code (pgcrypto suite 25/25).

I agree yours is simpler: no errno handling, no sign check, and
no duplicated ereport blocks. I will adopt it for v2.

One cosmetic difference I noticed: since the clamp NOTICEs now
print the token as supplied for every out-of-range value, a
non-canonical token changes text -- rounds=+10 and rounds=010
now report "rounds=+10" / "rounds=010" instead of "rounds=10".
That seems harmless, and arguably more accurate, but I wanted
to flag it, since v1 deliberately left the in-range messages
untouched.

Most of the similar postgresql code is just like
"if (errno == ERANGE)" instead of using another variable.

With the strtoi64 approach there is no errno check left at
all, so the rounds_overflow variable goes away with it.

We might be OK to turn negative rounds value to error since
negative rounds value does not make any sense.

I would prefer to keep clamping negative values in this patch.
Clamping is the long-standing behaviour for values that fit in
an int today -- rounds=-5 gives the below-minimum NOTICE and
1000 rounds on current master -- and this bug is specifically
about the silent modulo 2^32 wrap of values that do not fit.
Turning negative values into an ERROR would change behaviour
for inputs that currently work, and it would be hard to
backpatch, while the wrap fix itself should be backpatchable
to REL_18_STABLE. If negative rounds should error, I think
that deserves its own discussion and a master-only change.
What do you think?

I will post a v2 with the strtoi64 change and the regression
tests from v1 (unchanged) shortly.

Testing was done with AI assistance.
Regards,
Prashant Chinnam

#4Prashant Chinnam
crprashant@hotmail.com
In reply to: Prashant Chinnam (#3)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi Takeshi,

Thanks again for the review and the alternative patch.
Attached is v2, which adopts your strtoi64 approach as
discussed.

The crypt-sha.c change is the one you suggested: parse rounds=
with strtoi64() so out-of-range values saturate into the
existing clamping, and report the value as supplied in the
clamp NOTICEs. The regression tests are carried over from v1
unchanged.

Behaviour is identical to v1 on the full test battery,
including the values beyond int64 in both directions, and the
pgcrypto regression suite passes 25/25.

As noted before, sha-crypt support exists only in PG 18 and
master, so backpatching to REL_18_STABLE should be considered.

Testing was done with AI assistance.

Regards,
Prashant Chinnam

Attachments:

t254356_4
v2-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchapplication/octet-stream; name=v2-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchDownload+94-6
#5Chao Li
li.evan.chao@gmail.com
In reply to: Prashant Chinnam (#4)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

On Oct 27, 2026, at 09:13, Prashant Chinnam <crprashant@hotmail.com> wrote:

Hi Takeshi,

Thanks again for the review and the alternative patch.
Attached is v2, which adopts your strtoi64 approach as
discussed.

The crypt-sha.c change is the one you suggested: parse rounds=
with strtoi64() so out-of-range values saturate into the
existing clamping, and report the value as supplied in the
clamp NOTICEs. The regression tests are carried over from v1
unchanged.

Behaviour is identical to v1 on the full test battery,
including the values beyond int64 in both directions, and the
pgcrypto regression suite passes 25/25.

As noted before, sha-crypt support exists only in PG 18 and
master, so backpatching to REL_18_STABLE should be considered.

Testing was done with AI assistance.

Regards,
Prashant Chinnam<v2-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patch>

I think strtoi64() works properly here. It maps to strtol() or strtoll(), depending on the platform. According to the man page [1]https://man7.org/linux/man-pages/man3/strtol.3p.html, on overflow, it returns INT64_MAX or INT64_MIN. Both values are outside the supported range (PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX), so srounds will be clamped correctly.

Another thing the patch does is change the NOTICE message to print the original input string, which is good in most cases. A small concern is: what if the input string is unreasonably long? Maybe we should cap the maximum length of the printed string?

[1]: https://man7.org/linux/man-pages/man3/strtol.3p.html

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#6Prashant Chinnam
crprashant@hotmail.com
In reply to: Chao Li (#5)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi Chao,

Another thing the patch does is change the NOTICE message
to print the original input string, which is good in most
cases. A small concern is: what if the input string is
unreasonably long? Maybe we should cap the maximum length
of the printed string?

Good catch, thank you. I checked how far a token can
actually get: nothing on the way in bounds it. pg_crypt()
passes the salt text to px_crypt_shacrypt() unchecked, and
the only length limit in that function,
PX_SHACRYPT_SALT_MAX_LEN, applies to the salt proper, after
the rounds token. Measured on the v2 build, a token of
10000 digits produces a NOTICE of 10076 characters.

Attached is v3, which caps the printed token at 32
characters and appends "..." when it was truncated. 32 is
more than an int64 literal, sign included, can ever need,
so every value a strtoi64() result can hold still prints
in full; only hand-made overlong tokens are cut. With the
cap, the same 10000-digit token now produces a NOTICE of
111 characters. The parsing and clamping do not change,
the comparison battery output is identical to v2, and a
regression test with a 10000-character token is included;
the pgcrypto suite passes 25/25 (and the new test fails
against v2's code, on exactly the new NOTICE line).

Takeshi -- Cc'ing you because the %.*s form was yours;
the cap only limits how much of the token gets echoed.

Testing was done with AI assistance.

Regards,
Prashant Chinnam

Attachments:

t254356_6
v3-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchapplication/octet-stream; name=v3-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchDownload+135-6
#7Takeshi Ideriha
iderihatakeshi@gmail.com
In reply to: Prashant Chinnam (#3)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi Prashant

On 2026/10/27 9:39, Prashant Chinnam wrote:

I would prefer to keep clamping negative values in this patch.
Clamping is the long-standing behaviour for values that fit in
an int today -- rounds=-5 gives the below-minimum NOTICE and
1000 rounds on current master -- and this bug is specifically
about the silent modulo 2^32 wrap of values that do not fit.
Turning negative values into an ERROR would change behaviour
for inputs that currently work, and it would be hard to
backpatch, while the wrap fix itself should be backpatchable
to REL_18_STABLE. If negative rounds should error, I think
that deserves its own discussion and a master-only change.
What do you think?

Sure. Thank you for pointing this out. As you said, how to handle
negative value is a bit different topic from the main issue here. Let's
keep the current behavior in this thread unless another concern arise.

Regards,
Takeshi Ideriha

#8Takeshi Ideriha
iderihatakeshi@gmail.com
In reply to: Prashant Chinnam (#6)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi all

On 2026/10/28 3:40, Prashant Chinnam wrote:

Hi Chao,

Another thing the patch does is change the NOTICE message
to print the original input string, which is good in most
cases. A small concern is: what if the input string is
unreasonably long? Maybe we should cap the maximum length
of the printed string?

Good catch, thank you. I checked how far a token can
actually get: nothing on the way in bounds it. pg_crypt()
passes the salt text to px_crypt_shacrypt() unchecked, and
the only length limit in that function,
PX_SHACRYPT_SALT_MAX_LEN, applies to the salt proper, after
the rounds token. Measured on the v2 build, a token of
10000 digits produces a NOTICE of 10076 characters.

Attached is v3, which caps the printed token at 32
characters and appends "..." when it was truncated. 32 is
more than an int64 literal, sign included, can ever need,
so every value a strtoi64() result can hold still prints
in full; only hand-made overlong tokens are cut. With the
cap, the same 10000-digit token now produces a NOTICE of
111 characters. The parsing and clamping do not change,
the comparison battery output is identical to v2, and a
regression test with a 10000-character token is included;
the pgcrypto suite passes 25/25 (and the new test fails
against v2's code, on exactly the new NOTICE line).

Takeshi -- Cc'ing you because the %.*s form was yours;
the cap only limits how much of the token gets echoed.

Thank you for updating the patch. Why I used %.*s is I just wanted to
display a round value more than int64 as is. But an unreasonable long
round value might impact a workload just only for the notice message.
Limiting the digit for the message seems reasonable to me. I found
log_parameter_max_length_on_error parameter also controls the length of
a bind parameter with the same idea at appendStringInfoStringQuoted().

+#define PX_SHACRYPT_ROUNDS_PRINT_MAX 32

We could set a smaller value like 20, which is a digit of INT64_MIN. But
it's an arbitrary number and I don't have a strong opinion about this.

Regarding the test code, we can use repeat function instead of writing
so many zeros:

---
SELECT crypt('password', '$5$rounds=' || repeat('0', 33) || '5$saltsalt$')
;
NOTICE: 22003: rounds=00000000000000000000000000000000... is below
supported value (1000), using 1000 instead
LOCATION: px_crypt_shacrypt, crypt-sha.c:236
crypt
---------------------------------------------------------------------
$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9
(1 row)
---

Regards,
Takeshi Ideriha

#9Prashant Chinnam
crprashant@hotmail.com
In reply to: Takeshi Ideriha (#8)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi,

Attached is v4 of this patch.

As promised in the strtoint() thread, now that Tom's
commit 426551c8cf49 has landed and been backpatched
through PG 14 (REL_18_STABLE: ec65cbc00b2c), this
version switches the rounds= parse back to strtoint(),
relying on its new saturating behavior on overflow.

The delta against v3 is exactly one line in
crypt-sha.c (strtoi64() -> strtoint()); the clamping,
the NOTICE print cap, and all regression tests are
unchanged from v3. I verified it locally against
current master: the pgcrypto suite passes 25/25, and
the overflow battery and NOTICE probe output are
identical to v3.

Since the strtoint() change is backpatched, this
version should backpatch to REL_18_STABLE cleanly as
well.

Testing was done with AI assistance.

Regards,
Prashant Chinnam

Attachments:

t254356_9
v4-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchapplication/octet-stream; name=v4-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchDownload+134-5
#10Chao Li
li.evan.chao@gmail.com
In reply to: Prashant Chinnam (#9)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

On Oct 31, 2026, at 02:41, Prashant Chinnam <crprashant@hotmail.com> wrote:

Hi,

Attached is v4 of this patch.

As promised in the strtoint() thread, now that Tom's
commit 426551c8cf49 has landed and been backpatched
through PG 14 (REL_18_STABLE: ec65cbc00b2c), this
version switches the rounds= parse back to strtoint(),
relying on its new saturating behavior on overflow.

The delta against v3 is exactly one line in
crypt-sha.c (strtoi64() -> strtoint()); the clamping,
the NOTICE print cap, and all regression tests are
unchanged from v3. I verified it locally against
current master: the pgcrypto suite passes 25/25, and
the overflow battery and NOTICE probe output are
identical to v3.

Since the strtoint() change is backpatched, this
version should backpatch to REL_18_STABLE cleanly as
well.

Testing was done with AI assistance.

Regards,
Prashant Chinnam<v4-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patch>

Thanks for updating the patch. I reviewed and tested v4 today. I have a few comments:

1
```
#define PX_SHACRYPT_ROUNDS_PRINT_MAX 32
```

I think 32 characters is unnecessarily long for a NOTICE message. Since PX_SHACRYPT_ROUNDS_MAX (999999999) is only 9 digits long, I think a limit of 10 characters should be sufficient.

We could also define PX_SHACRYPT_ROUNDS_PRINT_MAX based on PX_SHACRYPT_ROUNDS_MAX, for example:
```
#define PX_SHACRYPT_ROUNDS_PRINT_MAX ((int) sizeof(CppAsString2(PX_SHACRYPT_ROUNDS_MAX)))
```

In this way, PX_SHACRYPT_ROUNDS_PRINT_MAX is 10, so the hint message is like:
```
evantest=# SELECT crypt('password', E'$5$rounds=1000000000000000000000000000000000000000000000000000000$saltsalt$');
NOTICE: rounds=1000000000... exceeds maximum supported value (999999999), using 999999999 instead
```

2
```
if (*endp != '$')
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
errmsg("could not parse salt options"));
```

This is a pre-existing issue, not a problem introduced by your patch. The code only checks whether endp points to $, so it doesn't reject an empty rounds value:
```
evantest=# SELECT crypt('password', E'$5$rounds=$saltsalt$');
NOTICE: rounds= is below supported value (1000), using 1000 instead
crypt
---------------------------------------------------------------------
$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9
(1 row)
```

When no digits are converted, strtoint() returns 0 and sets endp to num. Since num points to $, the existing check passes, and the value is clamped to 1000.

I think an empty rounds value should be rejected. We could handle this by checking whether any characters were consumed:
```
if (num == endp || *endp != '$')
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
errmsg("could not parse salt options"));
```

I'm fine if you prefer not to include this in the current patch. In that case, I can propose a separate patch.

3
```
SELECT crypt('password', '$5$rounds=0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005$saltsalt$')
= '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9' AS result;

```

This test containing 10,000 digits is rather long. I think we could simplify it using repeat(), for example:
```
SELECT crypt('password’, '$5$rounds=' || repeat('0', 9999) || '5$saltsalt$’)
= '$5$rounds=1000$saltsalt$azOwbpkvuuBKkE82dQPwTsQE8JyT9Fflpr9aKid3aT9’ AS result;
```

This would preserve the test coverage while making the SQL much shorter and easier to maintain.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#11Prashant Chinnam
crprashant@hotmail.com
In reply to: Chao Li (#10)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

Hi Chao,

Thanks for reviewing and testing v4. Replies inline.

I think 32 characters is unnecessarily long for a NOTICE
message. Since PX_SHACRYPT_ROUNDS_MAX (999999999) is
only 9 digits long, I think a limit of 10 characters
should be sufficient.

#define PX_SHACRYPT_ROUNDS_PRINT_MAX \
((int) sizeof(CppAsString2(PX_SHACRYPT_ROUNDS_MAX)))

Adopted exactly as you suggested, in the attached v5.
The define evaluates to 10, and I verified the boundary
behavior: a 10-character token still prints in full with
no ellipsis (for example rounds=4294968296, and INT_MAX
2147483647), while an 11-character token is truncated;
the INT_MIN token prints as "rounds=-214748364...". The
overlong-token case now shows:
NOTICE: rounds=0000000000... is below supported
value (1000), using 1000 instead

This is a pre-existing issue, not a problem introduced
by your patch. The code only checks whether endp
points to $, so it doesn't reject an empty rounds
value.

I'm fine if you prefer not to include this in the
current patch. In that case, I can propose a separate
patch.

Agreed, an empty rounds= silently clamping to 1000 does
not look right, and checking num == endp would fix it.
I would still prefer to keep it out of this patch: it is
a behavior change (a new ERROR for input that is accepted
today) beyond the reported bug, and I would rather not
widen the scope of a patch that is meant to be
backpatched to REL_18_STABLE. Please do propose it as
a separate patch, as you offered; it stands on its own
that way.

This test containing 10,000 digits is rather long. I
think we could simplify it using repeat(), for example:

Also adopted. The test now builds the token as
'$5$rounds=' || repeat('0', 9999) || '5$saltsalt$'
which produces byte-identical input (9999 zeros followed
by a 5), so the coverage is unchanged. As a side effect
the patch file shrinks from 32K to 12K.

v5 is attached. It is otherwise identical to v4: the
pgcrypto suite passes 25/25 locally, and the comparison
battery output differs from v4 only in the shortened
NOTICE echoes described above.

Testing was done with AI assistance.

Regards,
Prashant Chinnam

Attachments:

t254356_11
v5-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchapplication/octet-stream; name=v5-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patchDownload+135-5
#12Chao Li
li.evan.chao@gmail.com
In reply to: Prashant Chinnam (#11)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

On Nov 3, 2026, at 04:49, Prashant Chinnam <crprashant@hotmail.com> wrote:

This is a pre-existing issue, not a problem introduced
by your patch. The code only checks whether endp
points to $, so it doesn't reject an empty rounds
value.

I'm fine if you prefer not to include this in the
current patch. In that case, I can propose a separate
patch.

Agreed, an empty rounds= silently clamping to 1000 does
not look right, and checking num == endp would fix it.
I would still prefer to keep it out of this patch: it is
a behavior change (a new ERROR for input that is accepted
today) beyond the reported bug, and I would rather not
widen the scope of a patch that is meant to be
backpatched to REL_18_STABLE. Please do propose it as
a separate patch, as you offered; it stands on its own
that way.

No problem, I will propose a patch for that.

Regards,
Prashant Chinnam<v5-0001-pgcrypto-Fix-sha-crypt-rounds-values-that-do-not-fit.patch>

Thanks for updating the patch. I will review v5 today.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#13Chao Li
li.evan.chao@gmail.com
In reply to: Chao Li (#12)
Re: [PATCH] pgcrypto: Fix sha-crypt rounds= values that do not fit in int (BUG #19743)

On Nov 3, 2026, at 09:03, Chao Li <li.evan.chao@gmail.com> wrote:

Thanks for updating the patch. I will review v5 today.

V5 LGTM.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/