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.