From 102d53ea9e611adeaaf57d816622c702f7d0cd01 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 7 May 2026 20:29:11 +0200 Subject: [PATCH] Tighten CSR extensionRequest bounds Don't allow parts of a compound (SEQUENCE or SET) to go beyond the containing compound. The inline CSR regression test is derived from the existing "X509 CSR ASN.1 (OK)" DER test vector. In the extensionRequest attribute, the original well-formed fragment is: a029 3027 06092a864886f70d01090e 311a 3018 ... where 31 1a is the SET containing the Extensions sequence and 30 18 is the contained Extensions sequence. The malformed test changes only the SET length byte, from 1a to 19: a029 3027 06092a864886f70d01090e 3119 3018 ... The attribute SEQUENCE length and the inner Extensions sequence length are left unchanged. This makes the SET one byte too short for its containing attribute, so the parser must reject it with MBEDTLS_ERR_ASN1_LENGTH_MISMATCH instead of parsing the trailing byte as data outside the SET. There is no known security impact, just some risk reduction. Signed-off-by: Gilles Peskine --- .../x509-csr-extension-request-bounds.txt | 3 +++ library/x509_csr.c | 18 +++++++++++++++--- tests/suites/test_suite_x509parse.data | 12 ++++++++---- 3 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 ChangeLog.d/x509-csr-extension-request-bounds.txt diff --git a/ChangeLog.d/x509-csr-extension-request-bounds.txt b/ChangeLog.d/x509-csr-extension-request-bounds.txt new file mode 100644 index 0000000000..dfc706703c --- /dev/null +++ b/ChangeLog.d/x509-csr-extension-request-bounds.txt @@ -0,0 +1,3 @@ +Bugfix + * Reject malformed X.509 CSRs where the extensionRequest attribute + wrappers do not exactly contain the requested extensions. diff --git a/library/x509_csr.c b/library/x509_csr.c index 174ca6ffc3..4fad0738b4 100644 --- a/library/x509_csr.c +++ b/library/x509_csr.c @@ -227,18 +227,30 @@ static int x509_csr_parse_attributes(mbedtls_x509_csr *csr, /* Check that this is an extension-request attribute */ if (MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS9_CSR_EXT_REQ, &attr_oid) == 0) { - if ((ret = mbedtls_asn1_get_tag(p, end, &len, + if ((ret = mbedtls_asn1_get_tag(p, end_attr_data, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SET)) != 0) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret); } - if ((ret = mbedtls_asn1_get_tag(p, end, &len, + const unsigned char *end_set = *p + len; + if (end_set != end_attr_data) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + } + + if ((ret = mbedtls_asn1_get_tag(p, end_set, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret); } - if ((ret = x509_csr_parse_extensions(csr, p, *p + len, cb, p_ctx)) != 0) { + const unsigned char *end_exts = *p + len; + if (end_exts != end_set) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + } + + if ((ret = x509_csr_parse_extensions(csr, p, end_exts, cb, p_ctx)) != 0) { return ret; } diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index b5470a5067..314c175ca9 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -2951,6 +2951,10 @@ X509 CSR ASN.1 (OK) depends_on:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_ECP_HAVE_SECP256R1:MBEDTLS_MD_CAN_SHA1:!MBEDTLS_X509_REMOVE_INFO mbedtls_x509_csr_parse:"308201183081bf0201003034310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c31123010060355040313096c6f63616c686f73743059301306072a8648ce3d020106082a8648ce3d0301070342000437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edffa029302706092a864886f70d01090e311a301830090603551d1304023000300b0603551d0f0404030205e0300906072a8648ce3d04010349003046022100b49fd8c8f77abfa871908dfbe684a08a793d0f490a43d86fcf2086e4f24bb0c2022100f829d5ccd3742369299e6294394717c4b723a0f68b44e831b6e6c3bcabf97243":"CSR version \: 1\nsubject name \: C=NL, O=PolarSSL, CN=localhost\nsigned using \: ECDSA with SHA1\nEC key size \: 256 bits\n\nkey usage \: Digital Signature, Non Repudiation, Key Encipherment\n":0 +X509 CSR ASN.1 (extensionRequest SET length too short) +depends_on:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_ECP_HAVE_SECP256R1 +mbedtls_x509_csr_parse:"308201183081bf0201003034310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c31123010060355040313096c6f63616c686f73743059301306072a8648ce3d020106082a8648ce3d0301070342000437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edffa029302706092a864886f70d01090e3119301830090603551d1304023000300b0603551d0f0404030205e0300906072a8648ce3d04010349003046022100b49fd8c8f77abfa871908dfbe684a08a793d0f490a43d86fcf2086e4f24bb0c2022100f829d5ccd3742369299e6294394717c4b723a0f68b44e831b6e6c3bcabf97243":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_LENGTH_MISMATCH + X509 CSR ASN.1 (Unsupported critical extension, critical=true) depends_on:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_ECP_HAVE_SECP256R1:MBEDTLS_MD_CAN_SHA256:!MBEDTLS_X509_REMOVE_INFO mbedtls_x509_csr_parse:"308201233081cb02010030413119301706035504030c1053656c66207369676e65642074657374310b300906035504061302444531173015060355040a0c0e41757468437274444220546573743059301306072a8648ce3d020106082a8648ce3d03010703420004c11ebb9951848a436ca2c8a73382f24bbb6c28a92e401d4889b0c361f377b92a8b0497ff2f5a5f6057ae85f704ab1850bef075914f68ed3aeb15a1ff1ebc0dc6a028302606092a864886f70d01090e311930173015060b2b0601040183890c8622020101ff0403010101300a06082a8648ce3d040302034700304402200c4108fd098525993d3fd5b113f0a1ead8750852baf55a2f8e670a22cabc0ba1022034db93a0fcb993912adcf2ea8cb4b66389af30e264d43c0daea03255e45d2ccc":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_UNEXPECTED_TAG @@ -3086,17 +3090,17 @@ X509 CSR ASN.1 (attributes: invalid len (len > data)) depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C mbedtls_x509_csr_parse_file:"../framework/data_files/parse_input/test_csr_v3_all_malformed_attributes_len1.csr.der":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_OUT_OF_DATA -X509 CSR ASN.1 (attributes: invalid len (len < data)) +X509 CSR ASN.1 (attributes: extensionRequest SET extends past attribute) depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C -mbedtls_x509_csr_parse_file:"../framework/data_files/parse_input/test_csr_v3_all_malformed_attributes_len2.csr.der":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_LENGTH_MISMATCH +mbedtls_x509_csr_parse_file:"../framework/data_files/parse_input/test_csr_v3_all_malformed_attributes_len2.csr.der":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_OUT_OF_DATA X509 CSR ASN.1 (attributes: extension request invalid len (len > data)) depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C mbedtls_x509_csr_parse_file:"../framework/data_files/parse_input/test_csr_v3_all_malformed_attributes_extension_request_sequence_len1.csr.der":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_OUT_OF_DATA -X509 CSR ASN.1 (attributes: extension request invalid len (len < data)) +X509 CSR ASN.1 (attributes: extensionRequest sequence shorter than SET) depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C -mbedtls_x509_csr_parse_file:"../framework/data_files/parse_input/test_csr_v3_all_malformed_attributes_extension_request_sequence_len2.csr.der":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_OUT_OF_DATA +mbedtls_x509_csr_parse_file:"../framework/data_files/parse_input/test_csr_v3_all_malformed_attributes_extension_request_sequence_len2.csr.der":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_LENGTH_MISMATCH X509 CSR ASN.1 (extensions: invalid sequence tag) depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C