Hi, Just following up on the email I sent earlier regarding a potential integer overflow in decoding.c reported by Coverity. Your input would be really helpful as I am new to libtasn1. Below Is the Coverity report along with my preliminary analysis for your reference.
Thanks, Lidong > On Jul 1, 2025, at 3:54 PM, Lidong Chen <[email protected]> wrote: > > Hi, > > From the grub2 Coverity report, it raised a potential integer argument > overflow in asn1_der_decoding2() (CID 463073). > > ***** CID 463073 > 1481 DECR_LEN (ider_len, len2); > 1482 > 1483 len4 = > 1484 asn1_get_length_der (der + counter + len2, ider_len, > &len3); > > 1485 if (IS_ERR (len4, flags)) > 1486 { > 1487 result = ASN1_DER_ERROR; > 1488 warn (); > 1489 goto cleanup; > 1490 } > 1491 if (len4 != -1) /* definite */ > 1492 { > 433. overflow: The expression len2 is considered to have possibly > overflowed. > 1493 len2 += len4; > 1494 > 434. Condition ider_len < 0, taking false branch. > 1495 DECR_LEN (ider_len, len4 + len3); > 435. overflow: The expression len2 + len3 is deemed overflowed because > at least one of its arguments has overflowed. > > CID 463073: (#1 of 1): Overflowed integer argument (INTEGER_OVERFLOW) > 436. overflow_sink: len2 + len3, which might have underflowed, is passed to > _asn1_set_value_lv(p, der + counter, len2 + len3).[show details] > 1496 _asn1_set_value_lv (p, der + counter, len2 + len3); > ***** > > However, it seems to be false positive. > > DECR_LEN (ider_len, len2) at line 1481 subtracts len2 from ider_len. > The remaining ider_len is passed to asn1_get_length_der(), which ensures: > - len4 < INT_MAX > - len4 + len3 doesn’t overflow, > - len4 + len3 <= ider_len > > This implies that the sum len2 + len3 + len4 is bounded by ider_len. > Therefore, > the argument, len2 + len3, passed to _asn1_set_value_lv() is within safe > bound. > > Since I'm not familiar with libtasn1, I'd need libtasn1 upstream to confirm > that. > > Thanks, > Lidong >
