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