Hi, This patch is derived from another patch [1] I just reviewed.
Patch [1] changes pgcrypto's crypt() to print the raw input string when
clamping happens. When the rounds= option is given an empty string, strtoint()
returns 0 and sets endp to point to the start of the string to indicate that
nothing was parsed. The current implementation doesn't check that, so an empty
input is treated as 0 and clamped to 1000. Patch [1] makes this behavior more
visible:
```
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)
```
The notice message says an empty value is below 1000, which looks odd. I raised
this in my review of patch [1], but the author considered it an existing issue
and wanted to keep the patch scope focused. So I'm proposing a fix in this
separate thread.
Also, the SHA-crypt spec describes the rounds value as an unsigned decimal
number, see [2]:
```
If the SALT strings starts with
rounds=<N>$
where N is an unsigned decimal number the numeric value of N is used
to modify the algorithm used.
```
The sample code in [2] also accepts an empty value and clamps it to 1000.
However, PG already differs from that sample code in handling malformed rounds
options: when endp doesn't point to '$', PG reports an error, while the sample
code uses the malformed rounds option itself as the salt. So, rejecting an
empty rounds value would be consistent with our existing validation.
[1] https://postgr.es/m/[email protected]
[2] https://www.akkadia.org/drepper/SHA-crypt.txt
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v1-0001-Reject-empty-rounds-values-in-pgcrypto-s-SHA-cryp.patch
Description: Binary data
