Hi Prasad,
On 2026-10-01T07:16:20, Prasad Kummari <[email protected]> wrote:
> rsa: fix undersized modulus buffers in rsa_gen_key_prop()
>
> n and rr are sized with a floor division by 32 (num_bits >> 5), but
> br_i32_decode() and the zero-init loop before br_i32_reduce() need
> ceil(bits/32) words plus a header word. Whenever the stripped modulus
> byte length isn't a multiple of 4, br_i32_decode() writes one word
> past the end of n; whenever it's odd, the zero-init loop writes one
> zero word past the end of rr. The modulus length is fully
> attacker-controlled via the DER public key's ASN.1 INTEGER length, so
> both overflows are reachable with a crafted key.
>
> Switch both allocations to ceiling division so they always hold the
> full word count each site writes. rrtmp is already exactly sized and
> is left unchanged.
>
> Fixes: 49d0ea3746ca ("lib: rsa: fix allocated size for rr and rrtmp in
> rsa_gen_key_prop()")
> Signed-off-by: Prasad Kummari <[email protected]>
>
> lib/rsa/rsa-keyprop.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
> @@ -687,8 +687,8 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen,
> struct key_prop **prop)
> - n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
> - rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits * 2) >> 5));
> + n = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits + 31) >> 5));
> + rr = calloc(sizeof(uint32_t), 1 + ((((*prop)->num_bits * 2) + 31) >>
> 5));
The rr line is now over 80 columns and the brackets are hard to
follow. Please can you use DIV_ROUND_UP(), as used for mlen in
br_i32_reduce():
n = calloc(sizeof(uint32_t), 1 + DIV_ROUND_UP((*prop)->num_bits, 32));
rr = calloc(sizeof(uint32_t), 1 +
DIV_ROUND_UP((*prop)->num_bits * 2, 32));
BTW there is a related problem. rsa_get_n() only rejects a zero-length
INTEGER, so an all-zero modulus (e.g. a single 0x00 byte) leaves
num_bits as 0. Then n is a single word, br_i32_ninv32(n[1]) reads past
the end, and modulus is malloc(0). Please can you reject num_bits == 0
with -EINVAL after stripping the leading zeroes, perhaps in a separate
patch?
Reviewed-by: Simon Glass <[email protected]>
Regards,
Simon
