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

On 6/18/25 11:53, Paul Eggert wrote:
> 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.

I was about to say that I thought amd64 would trip that static assert, 
since "int" is 32-bit but "long int" and "size_t" are 64-bit.

> This looks like a real bug, at least just from local inspection.

So the checks are indeed incorrect...

>> 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.

... and the checks are also needed.  "Fun."


-- Jacob



_______________________________________________
Gcrypt-devel mailing list
[email protected]
https://lists.gnupg.org/mailman/listinfo/gcrypt-devel