x509_crl_parse: fix 1-byte buffer overflow and entry->raw.tag
In the entries (mbedtls_x509_crl_entry values) on the list constructed by mbedtls_x509_crl_parse_der(), set entry->raw.tag to (SEQUENCE | CONSTRUCTED) rather than to the tag of the first ASN.1 element of the entry (which happens to be the tag of the serial number, so INTEGER or INTEGER | CONTEXT_SPECIFIC). This is doesn't really matter in practice (and in particular the value is never used in Mbed TLS itself), and isn't documented, but at least it's consistent with how mbedtls_x509_buf is normally used. The primary importance of this change is that the old code tried to access the tag of the first element of the entry even when the entry happened to be empty. If the entry was empty and not followed by anything else in the CRL, this could cause a read 1 byte after the end of the buffer containing the CRL. The test case "X509 CRL ASN1 (TBSCertList, single empty entry at end)" hit the problematic buffer overflow, which is detected with ASan. Credit to OSS-Fuzz for detecting the problem. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
parent
b2281e1cf0
commit
5dd5a491da
2 changed files with 5 additions and 1 deletions
4
ChangeLog.d/x509parse_crl-empty_entry.txt
Normal file
4
ChangeLog.d/x509parse_crl-empty_entry.txt
Normal file
|
@ -0,0 +1,4 @@
|
||||||
|
Security
|
||||||
|
* Fix a 1-byte buffer overread in mbedtls_x509_crl_parse_der().
|
||||||
|
Credit to OSS-Fuzz for detecting the problem and to Philippe Antoine
|
||||||
|
for pinpointing the problematic code.
|
|
@ -259,13 +259,13 @@ static int x509_get_entries( unsigned char **p,
|
||||||
size_t len2;
|
size_t len2;
|
||||||
const unsigned char *end2;
|
const unsigned char *end2;
|
||||||
|
|
||||||
|
cur_entry->raw.tag = **p;
|
||||||
if( ( ret = mbedtls_asn1_get_tag( p, end, &len2,
|
if( ( ret = mbedtls_asn1_get_tag( p, end, &len2,
|
||||||
MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED ) ) != 0 )
|
MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED ) ) != 0 )
|
||||||
{
|
{
|
||||||
return( ret );
|
return( ret );
|
||||||
}
|
}
|
||||||
|
|
||||||
cur_entry->raw.tag = **p;
|
|
||||||
cur_entry->raw.p = *p;
|
cur_entry->raw.p = *p;
|
||||||
cur_entry->raw.len = len2;
|
cur_entry->raw.len = len2;
|
||||||
end2 = *p + len2;
|
end2 = *p + len2;
|
||||||
|
|
Loading…
Reference in a new issue