← Back to context

Comment by tialaramex

4 hours ago

The actual vulnerability is indeed the copy. What we used to do is this:

1. Find out how big this data is, we tell the ASN.1 code how big it's allowed to be, but since we're not storing it anywhere those tests don't matter

2. Check we found at least some data, zero isn't OK, failure isn't OK, but too big is fine

3. Copy the too big data onto a local buffer

The API design is typical of C and has the effect of encouraging this mistake

    int ossl_asn1_type_get_octetstring_int(const ASN1_TYPE *a, long *num, unsigned char *data, int max_len)

That "int" we're returning is either -1 or the claimed length of the ASN.1 data without regard to how long that is or whether it makes sense.

This encourages people to either forget the return value entirely (it's just some integer, who cares, in the happy path this works) or check it for -1 which indicates some fatal ASN.1 layer problem, give up, but ignore other values.

If the thing you got back from your function was a Result type you'd know that this wasn't OK, because it isn't OK. But the "Eh, everything is an integer" model popular in C discourages such sensible choices because they were harder to implement decades ago.