Re: [PATCH] cipher:pk:sexp: Remove bogus checks in SEXP parsing.

Paul Eggert via Gcrypt-devel <[email protected]> Wed, 18 Jun 2025 09:53:30 -0700
Newsgroups gmane.comp.encryption.gpg.libgcrypt.devel
Organization UCLA Computer Science Department
Message-ID <[email protected]>
On 2025-06-17 20:27, Jacob Bachmeyer via Gcrypt-devel wrote:
>> It is a size_t so it is unsigned.
> 
> Then the checks are correct as written.

Not on oddball platforms where SIZE_MAX <= INT_MAX, because in that case 
it's signed integer overflow and behavior is undefined. POSIX allows 
such platforms.

Maybe gcrypt should have a static_assert (INT_MAX < SIZE_MAX)? That 
might be easier than adjusting all its size_t-calculating code to be 
portable to oddball platforms. Unless there's a goal to be fully POSIX 
portable.

More important, the checks are not valid on platforms like x86-64 where 
UINT_MAX < SIZE_MAX, because mpi_set_opaque takes an unsigned int arg.
This looks like a real bug, at least just from local inspection.


> I advocate for defense-in-depth

Although this defense is needed it's not defense in depth, as there's no 
guarantee malloc will fail with sizes close to SIZE_MAX.

However, it's better to use ckd_mul than to try do do multiplication 
overflow checks by hand, and the code would be better if it did that. 
You can use Gnulib's stdckdint module to support ckd_mul on pre-C23 
platforms.