From 0d1db20490633e53ebcf9eadc6fce9b1eb2f5f4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 30 Jul 2019 14:11:25 +0200 Subject: [PATCH] Fix bug in skip_date() (MBEDTLS_X509_CRT_REMOVE_TIME) Asserting `*p == end` right after setting `end = *p + len` will always fail unless `len == 0`, which is never the case with properly-formed certificates. The function x509_skip_dates() is modelled after x509_get_dates() which between setting `end` and comparing it to `*p` calls mbedtls_x509_get_time() which advances `*p` to the expected value, which is why this test works in get_dates(). Since `skip_dates()` has `skip`, not `validate` in its name, and the entire point of `MBEDTLS_X509_CRT_REMOVE_TIME` is to save code, we don't want to call the relatively large functions needed to properly parse (and validate) dates before throwing the parsed dates away, we can just fast-forward to the end of the sequence. This makes updating `end` and comparing it to `*p` after the fast-forward redundant, as the comparison will always be true (unlike the case where we actually parse the contents of the sequence). This bug was found by `all.sh test_baremetal` - no need for a new test. --- library/x509_crt.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 4e5ff43bd..7a667cdee 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -743,11 +743,8 @@ static int x509_skip_dates( unsigned char **p, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_DATE + ret ); - end = *p + len; - - if( *p != end ) - return( MBEDTLS_ERR_X509_INVALID_DATE + - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH ); + /* skip contents of the sequence */ + *p += len; return( 0 ); }