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

Jacob Bachmeyer via Gcrypt-devel <[email protected]> Tue, 17 Jun 2025 22:27:11 -0500
Newsgroups gmane.comp.encryption.gpg.libgcrypt.devel
Message-ID <[email protected]>
On 6/17/25 21:20, Collin Funk wrote:
> Jacob Bachmeyer via Gcrypt-devel <[email protected]> writes:
>
>> NACK!  NACK!  NACK!
>>
>> Those are checks for integer overflow.  (They may even be correct if
>> VALUELEN is unsigned.)
>>
>> If VALUELEN is a signed type, they are undefined behavior and need to
>> be fixed, not removed.
> It is a size_t so it is unsigned.

Then the checks are correct as written.

> But it looks harmless to remove to me. The call to 'malloc' in
> '_gcry_sexp_nth_buffer' will fail before VALUELEN overflows.

I advocate for defense-in-depth, especially in software as 
security-critical as GPG.  Nor are these expensive checks:  one 
multiplication and one comparison are very cheap compared to public key 
operations.


-- Jacob



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