From fe60132305df0eea16f90aeb8867fe344642a1ca Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Thu, 5 Apr 2018 16:53:35 +0200 Subject: [PATCH 1/5] Move a buffer size test before the first relevant read --- library/x509_crt.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index afff4e18bf..0885c8e3b7 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -574,6 +574,9 @@ static int x509_get_crt_ext( unsigned char **p, end_ext_data = *p + len; /* Get extension ID */ + if( ( end - *p ) < 1 ) + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + + MBEDTLS_ERR_ASN1_OUT_OF_DATA ); extn_oid.tag = **p; if( ( ret = mbedtls_asn1_get_tag( p, end, &extn_oid.len, MBEDTLS_ASN1_OID ) ) != 0 ) @@ -582,10 +585,6 @@ static int x509_get_crt_ext( unsigned char **p, extn_oid.p = *p; *p += extn_oid.len; - if( ( end - *p ) < 1 ) - return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + - MBEDTLS_ERR_ASN1_OUT_OF_DATA ); - /* Get optional critical */ if( ( ret = mbedtls_asn1_get_bool( p, end_ext_data, &is_critical ) ) != 0 && ( ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) ) From 470dfbabb9812ac056dd0ace4dcca33241bb3b0c Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Thu, 28 Jun 2018 16:23:39 +0200 Subject: [PATCH 2/5] Simplify OID tag parsing in x509_get_cert_ext( ) --- library/x509_crt.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 0885c8e3b7..ca8b4649e6 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -574,14 +574,10 @@ static int x509_get_crt_ext( unsigned char **p, end_ext_data = *p + len; /* Get extension ID */ - if( ( end - *p ) < 1 ) - return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + - MBEDTLS_ERR_ASN1_OUT_OF_DATA ); - extn_oid.tag = **p; - if( ( ret = mbedtls_asn1_get_tag( p, end, &extn_oid.len, MBEDTLS_ASN1_OID ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + extn_oid.tag = MBEDTLS_ASN1_OID; extn_oid.p = *p; *p += extn_oid.len; From dcae78a7a9c0fd83bb26f18e30a19551b132c62f Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Thu, 28 Jun 2018 16:32:54 +0200 Subject: [PATCH 3/5] Make a buffer limit more specific --- library/x509_crt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index ca8b4649e6..493d6334f8 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -574,7 +574,8 @@ static int x509_get_crt_ext( unsigned char **p, end_ext_data = *p + len; /* Get extension ID */ - if( ( ret = mbedtls_asn1_get_tag( p, end, &extn_oid.len, MBEDTLS_ASN1_OID ) ) != 0 ) + if( ( ret = mbedtls_asn1_get_tag( p, end_ext_data, &extn_oid.len, + MBEDTLS_ASN1_OID ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); extn_oid.tag = MBEDTLS_ASN1_OID; From 6ca436a4576c6b3a02c05fbfaefd159fd999daf2 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Mon, 16 Jul 2018 12:20:10 +0200 Subject: [PATCH 4/5] Update change log --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index 9ee82c6853..6aeacf1289 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,10 @@ Security a non DER-compliant certificate correctly signed by a trusted CA, or a trusted CA with a non DER-compliant certificate. Found by luocm on GitHub. Fixes #825. + * Fix an issue in the X.509 module which could lead to a buffer overread + during certificate extensions parsing. In case of receiving malformed + input (extensions length field equal to 0), an illegal read of one byte + beyond the input buffer is made. Found and analyzed by Nathan Crandall. Features * Add option MBEDTLS_AES_FEWER_TABLES to dynamically compute 3/4 of the AES tables From 463928a74b05219eb42f130eb94e1ea7a0d16821 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 24 Jul 2018 12:50:59 +0200 Subject: [PATCH 5/5] Fix code formatting --- library/x509_crt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 493d6334f8..2e7701d4fa 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -575,10 +575,10 @@ static int x509_get_crt_ext( unsigned char **p, /* Get extension ID */ if( ( ret = mbedtls_asn1_get_tag( p, end_ext_data, &extn_oid.len, - MBEDTLS_ASN1_OID ) ) != 0 ) + MBEDTLS_ASN1_OID ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); - extn_oid.tag = MBEDTLS_ASN1_OID; + extn_oid.tag = MBEDTLS_ASN1_OID; extn_oid.p = *p; *p += extn_oid.len; @@ -729,7 +729,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * memcpy( p, buf, crt->raw.len ); - // Direct pointers to the new buffer + // Direct pointers to the new buffer p += crt->raw.len - len; end = crt_end = p + len;