Skip to content

Commit 1feb8e6

Browse files
DemiMarieashishverma2691
authored andcommitted
fix(auth): properly validate X.509 extensions
get_ext() does not check the return value of the various mbedtls_* functions, as cert_parse() is assumed to have guaranteed that they will always succeed. However, it passes the end of an extension as the end pointer to these functions, whereas cert_parse() passes the end of the TBSCertificate. Furthermore, cert_parse() does *not* check that the contents of the extension have the same length as the extension itself. Before fd37982 ("fix(auth): forbid junk after extensions"), cert_parse() also does not check that the extension block extends to the end of the TBSCertificate. This is a problem, as mbedtls_asn1_get_tag() leaves *p and *len undefined on failure. In practice, this results in get_ext() continuing to parse at different offsets than were used (and validated) by cert_parse(), which means that the in-bounds guarantee provided by cert_parse() no longer holds. This patch fixes the remaining flaw by enforcing that the contents of an extension are the same length as the extension itself. Change-Id: Id4570f911402e34d5d6c799ae01a01f184c68d7c Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
1 parent 7e2103b commit 1feb8e6

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

drivers/auth/mbedtls/mbedtls_x509_parser.c

+12-6
Original file line numberDiff line numberDiff line change
@@ -329,33 +329,39 @@ static int cert_parse(void *img, unsigned int img_len)
329329
* in the boot chain.
330330
*/
331331
do {
332+
unsigned char *end_ext_data;
333+
332334
ret = mbedtls_asn1_get_tag(&p, end, &len,
333335
MBEDTLS_ASN1_CONSTRUCTED |
334336
MBEDTLS_ASN1_SEQUENCE);
335337
if (ret != 0) {
336338
return IMG_PARSER_ERR_FORMAT;
337339
}
340+
end_ext_data = p + len;
338341

339342
/* Get extension ID */
340-
ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_OID);
343+
ret = mbedtls_asn1_get_tag(&p, end_ext_data, &len, MBEDTLS_ASN1_OID);
341344
if (ret != 0) {
342345
return IMG_PARSER_ERR_FORMAT;
343346
}
344347
p += len;
345348

346349
/* Get optional critical */
347-
ret = mbedtls_asn1_get_bool(&p, end, &is_critical);
350+
ret = mbedtls_asn1_get_bool(&p, end_ext_data, &is_critical);
348351
if ((ret != 0) && (ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG)) {
349352
return IMG_PARSER_ERR_FORMAT;
350353
}
351354

352-
/* Data should be octet string type */
353-
ret = mbedtls_asn1_get_tag(&p, end, &len,
355+
/*
356+
* Data should be octet string type and must use all bytes in
357+
* the Extension.
358+
*/
359+
ret = mbedtls_asn1_get_tag(&p, end_ext_data, &len,
354360
MBEDTLS_ASN1_OCTET_STRING);
355-
if (ret != 0) {
361+
if ((ret != 0) || ((p + len) != end_ext_data)) {
356362
return IMG_PARSER_ERR_FORMAT;
357363
}
358-
p += len;
364+
p = end_ext_data;
359365
} while (p < end);
360366

361367
if (p != end) {

0 commit comments

Comments
 (0)