Skip to content

ASN template depth fix #11404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SparkiDev wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ASN template depth fix #11404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions tests/api/test_asn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2415,6 +2415,116 @@ int test_DecodeCertExtensions_empty_certpol_trailing(void)
return EXPECT_RESULT();
}

/* A constructed ASN.1 item must be exactly consumed by the items parsed out of
* it. GetASN_Items() used to only enforce that for containers still open when
* the template ran out, so a container the template walked into and back out of
* mid-way could declare a length longer than its contents and have the excess
* silently ignored.
*
* For a certificate that means TBSCertificate.validity can claim a length that
* runs past notBefore/notAfter and swallows subject, subjectPublicKeyInfo and
* the start of extensions. wolfSSL resumed parsing right after notAfter and
* accepted the certificate, while a strict parser rejects it and a lax one may
* resume after the declared end of validity and read a completely different
* subjectPublicKeyInfo. That disagreement is exploitable: a peer holding only
* the key at the real SPKI offset can satisfy an application's public-key pin
* for an unrelated key embedded in an extension.
*
* The two DERs below differ in exactly one byte: the length octet of the
* validity SEQUENCE, 0x1e (30, the correct length) versus 0x59 (89). */
int test_ParseCert_validity_length_overrun(void)
{
EXPECT_DECLS;

#if !defined(NO_CERTS) && !defined(NO_ASN) && defined(HAVE_ED25519) && \
defined(HAVE_ED25519_VERIFY)
/* Ed25519 certificate, empty issuer and subject, one unknown extension
* whose payload happens to be a second, well-formed SPKI. Well-formed:
* validity SEQUENCE at offset 43 has length 30 and holds exactly the two
* UTCTIMEs at offsets 45 and 60. */
static const byte validCert[] = {
0x30, 0x82, 0x01, 0x19, 0x30, 0x81, 0xcc, 0xa0, 0x03, 0x02, 0x01, 0x02,
0x02, 0x14, 0x3c, 0x4a, 0xc9, 0xfc, 0x05, 0xa5, 0x6c, 0xaa, 0x52, 0x8a,
0x71, 0xcb, 0xfc, 0xd4, 0xd1, 0x6e, 0x29, 0x8d, 0x6f, 0x01, 0x30, 0x05,
0x06, 0x03, 0x2b, 0x65, 0x70, 0x30, 0x00,
/* validity SEQUENCE, correct length 30 */
0x30, 0x1e, 0x17, 0x0d, 0x32,
0x36, 0x30, 0x39, 0x30, 0x32, 0x30, 0x34, 0x35, 0x34, 0x32, 0x36, 0x5a,
0x17, 0x0d, 0x32, 0x36, 0x31, 0x30, 0x30, 0x32, 0x30, 0x34, 0x35, 0x34,
0x32, 0x36, 0x5a, 0x30, 0x00, 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b,
0x65, 0x70, 0x03, 0x21, 0x00, 0x5e, 0x1b, 0x44, 0xb7, 0xe3, 0x7b, 0x3d,
0xe7, 0x24, 0x66, 0x20, 0xaf, 0x31, 0x66, 0x61, 0xaf, 0x52, 0x68, 0xa2,
0x62, 0xf4, 0x47, 0xe5, 0x93, 0x1c, 0x3e, 0xae, 0xab, 0xbc, 0x71, 0xca,
0xc2, 0xa3, 0x58, 0x30, 0x56, 0x30, 0x35, 0x06, 0x03, 0x2a, 0x03, 0x04,
0x04, 0x2e, 0x30, 0x00, 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65,
0x70, 0x03, 0x21, 0x00, 0xf3, 0xf9, 0x4d, 0x74, 0x9e, 0xd9, 0x61, 0xed,
0xfd, 0xb7, 0x9b, 0x6c, 0xff, 0x1e, 0x1c, 0xf5, 0xeb, 0x1b, 0x72, 0xcd,
0xc5, 0x8c, 0xfc, 0x4e, 0x71, 0xcd, 0x82, 0x9c, 0x13, 0x16, 0xc4, 0xfb,
0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e, 0x04, 0x16, 0x04, 0x14, 0x7a,
0x6a, 0xa2, 0xfd, 0x2e, 0xb6, 0x62, 0x22, 0x98, 0x18, 0x0b, 0x2d, 0x17,
0xe1, 0xf3, 0x6b, 0x86, 0x01, 0x50, 0xb1, 0x30, 0x05, 0x06, 0x03, 0x2b,
0x65, 0x70, 0x03, 0x41, 0x00, 0x0b, 0x53, 0x43, 0x87, 0x19, 0xc2, 0x2f,
0x7d, 0x3c, 0x32, 0x8a, 0x7f, 0xfe, 0xbb, 0xd7, 0x2c, 0x27, 0xd8, 0x59,
0x5b, 0x47, 0x3e, 0x5d, 0xd1, 0xc9, 0x63, 0x9b, 0x83, 0x74, 0x24, 0x75,
0x3b, 0xdd, 0x8a, 0xe1, 0x59, 0xe8, 0xcd, 0xff, 0xd0, 0x09, 0xee, 0xc8,
0x4c, 0xe2, 0x7c, 0x69, 0x49, 0xc2, 0xb3, 0xb2, 0x60, 0x7f, 0x9c, 0x6b,
0x0e, 0x0c, 0x97, 0x64, 0xd7, 0x6e, 0x9b, 0x4e, 0x00
};
/* Byte-for-byte identical except offset 44: the validity SEQUENCE declares
* length 89, so it runs to offset 134 and covers subject (75),
* subjectPublicKeyInfo (77) and the head of the extensions (121). */
static const byte overrunCert[] = {
0x30, 0x82, 0x01, 0x19, 0x30, 0x81, 0xcc, 0xa0, 0x03, 0x02, 0x01, 0x02,
0x02, 0x14, 0x3c, 0x4a, 0xc9, 0xfc, 0x05, 0xa5, 0x6c, 0xaa, 0x52, 0x8a,
0x71, 0xcb, 0xfc, 0xd4, 0xd1, 0x6e, 0x29, 0x8d, 0x6f, 0x01, 0x30, 0x05,
0x06, 0x03, 0x2b, 0x65, 0x70, 0x30, 0x00,
/* validity SEQUENCE, overrunning length 89 */
0x30, 0x59, 0x17, 0x0d, 0x32,
0x36, 0x30, 0x39, 0x30, 0x32, 0x30, 0x34, 0x35, 0x34, 0x32, 0x36, 0x5a,
0x17, 0x0d, 0x32, 0x36, 0x31, 0x30, 0x30, 0x32, 0x30, 0x34, 0x35, 0x34,
0x32, 0x36, 0x5a, 0x30, 0x00, 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b,
0x65, 0x70, 0x03, 0x21, 0x00, 0x5e, 0x1b, 0x44, 0xb7, 0xe3, 0x7b, 0x3d,
0xe7, 0x24, 0x66, 0x20, 0xaf, 0x31, 0x66, 0x61, 0xaf, 0x52, 0x68, 0xa2,
0x62, 0xf4, 0x47, 0xe5, 0x93, 0x1c, 0x3e, 0xae, 0xab, 0xbc, 0x71, 0xca,
0xc2, 0xa3, 0x58, 0x30, 0x56, 0x30, 0x35, 0x06, 0x03, 0x2a, 0x03, 0x04,
0x04, 0x2e, 0x30, 0x00, 0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65,
0x70, 0x03, 0x21, 0x00, 0xf3, 0xf9, 0x4d, 0x74, 0x9e, 0xd9, 0x61, 0xed,
0xfd, 0xb7, 0x9b, 0x6c, 0xff, 0x1e, 0x1c, 0xf5, 0xeb, 0x1b, 0x72, 0xcd,
0xc5, 0x8c, 0xfc, 0x4e, 0x71, 0xcd, 0x82, 0x9c, 0x13, 0x16, 0xc4, 0xfb,
0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e, 0x04, 0x16, 0x04, 0x14, 0x7a,
0x6a, 0xa2, 0xfd, 0x2e, 0xb6, 0x62, 0x22, 0x98, 0x18, 0x0b, 0x2d, 0x17,
0xe1, 0xf3, 0x6b, 0x86, 0x01, 0x50, 0xb1, 0x30, 0x05, 0x06, 0x03, 0x2b,
0x65, 0x70, 0x03, 0x41, 0x00, 0x0b, 0x53, 0x43, 0x87, 0x19, 0xc2, 0x2f,
0x7d, 0x3c, 0x32, 0x8a, 0x7f, 0xfe, 0xbb, 0xd7, 0x2c, 0x27, 0xd8, 0x59,
0x5b, 0x47, 0x3e, 0x5d, 0xd1, 0xc9, 0x63, 0x9b, 0x83, 0x74, 0x24, 0x75,
0x3b, 0xdd, 0x8a, 0xe1, 0x59, 0xe8, 0xcd, 0xff, 0xd0, 0x09, 0xee, 0xc8,
0x4c, 0xe2, 0x7c, 0x69, 0x49, 0xc2, 0xb3, 0xb2, 0x60, 0x7f, 0x9c, 0x6b,
0x0e, 0x0c, 0x97, 0x64, 0xd7, 0x6e, 0x9b, 0x4e, 0x00
};
DecodedCert cert;

/* The two encodings differ in the one length octet and nothing else. */
ExpectIntEQ(sizeof(validCert), sizeof(overrunCert));
ExpectIntEQ(validCert[44], 0x1e);
ExpectIntEQ(overrunCert[44], 0x59);
ExpectIntEQ(XMEMCMP(validCert, overrunCert, 44), 0);
ExpectIntEQ(XMEMCMP(validCert + 45, overrunCert + 45,
sizeof(validCert) - 45), 0);

/* Sanity check: the well-formed encoding parses. */
wc_InitDecodedCert(&cert, validCert, (word32)sizeof(validCert), NULL);
ExpectIntEQ(wc_ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL), 0);
wc_FreeDecodedCert(&cert);

/* The overrunning validity length must be rejected. */
wc_InitDecodedCert(&cert, overrunCert, (word32)sizeof(overrunCert), NULL);
ExpectIntEQ(wc_ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL),
WC_NO_ERR_TRACE(ASN_PARSE_E));
wc_FreeDecodedCert(&cert);
#endif
return EXPECT_RESULT();
}

int test_ParseCert_SM3wSM2_short_pubkey(void)
{
EXPECT_DECLS;
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_asn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ int test_DecodeCertExtensions_dup_certpol(void);
int test_DecodeCertExtensions_empty_certpol(void);
int test_DecodeCertExtensions_certpol_trailing_junk(void);
int test_DecodeCertExtensions_empty_certpol_trailing(void);
int test_ParseCert_validity_length_overrun(void);
int test_ParseCert_SM3wSM2_short_pubkey(void);
int test_ParseCert_dnBufferBoundary(void);
int test_wc_DecodeObjectId(void);
Expand Down Expand Up @@ -69,6 +70,7 @@ int test_wc_AsnFeatureCoverage(void);
TEST_DECL_GROUP("asn", test_DecodeCertExtensions_empty_certpol), \
TEST_DECL_GROUP("asn", test_DecodeCertExtensions_certpol_trailing_junk), \
TEST_DECL_GROUP("asn", test_DecodeCertExtensions_empty_certpol_trailing), \
TEST_DECL_GROUP("asn", test_ParseCert_validity_length_overrun), \
TEST_DECL_GROUP("asn", test_ParseCert_SM3wSM2_short_pubkey), \
TEST_DECL_GROUP("asn", test_ParseCert_dnBufferBoundary), \
TEST_DECL_GROUP("asn", test_wc_DecodeObjectId), \
Expand Down
9 changes: 6 additions & 3 deletions tests/api/test_mldsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1471,9 +1471,12 @@ int test_mldsa_check_key(void)

#if defined(WOLFSSL_HAVE_MLDSA) && \
defined(WOLFSSL_MLDSA_PUBLIC_KEY)
/* The AlgorithmIdentifier SEQUENCE holds only the algorithm OID, so its
* length covers those 11 bytes and no more - the outer SEQUENCE length is
* encoded on that basis. */
static const unsigned char ml_dsa_public_der[] = {
#ifndef WOLFSSL_NO_ML_DSA_44
0x30, 0x82, 0x05, 0x32, 0x30, 0x0d, 0x06, 0x09,
0x30, 0x82, 0x05, 0x32, 0x30, 0x0b, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03,
0x11, 0x03, 0x82, 0x05, 0x21, 0x00,
0xBC, 0x5F, 0xF8, 0x10, 0xEB, 0x08, 0x90, 0x48,
Expand Down Expand Up @@ -1641,7 +1644,7 @@ static const unsigned char ml_dsa_public_der[] = {
0xD8, 0x6D, 0xCA, 0x6B, 0xCD, 0x3D, 0x03, 0x8F,
0x9D, 0x3A, 0x7B, 0x66, 0xCB, 0xC7, 0xDF, 0x34
#elif !defined(WOLFSSL_NO_ML_DSA_65)
0x30, 0x82, 0x07, 0xb2, 0x30, 0x0d, 0x06, 0x09,
0x30, 0x82, 0x07, 0xb2, 0x30, 0x0b, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03,
0x12, 0x03, 0x82, 0x07, 0xa1, 0x00,
0xD2, 0xFD, 0x03, 0xF3, 0xA1, 0xB7, 0xF6, 0x35,
Expand Down Expand Up @@ -1889,7 +1892,7 @@ static const unsigned char ml_dsa_public_der[] = {
0x1A, 0xE7, 0x97, 0xF5, 0x6C, 0x63, 0x74, 0xBE,
0x0C, 0x79, 0x8C, 0x0C, 0xF3, 0x98, 0xF1, 0xED
#else
0x30, 0x82, 0x0a, 0x32, 0x30, 0x0d, 0x06, 0x09,
0x30, 0x82, 0x0a, 0x32, 0x30, 0x0b, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03,
0x13, 0x03, 0x82, 0x0a, 0x21, 0x00,
0x69, 0x24, 0xBB, 0x42, 0x57, 0xA7, 0xB9, 0xAF,
Expand Down
31 changes: 26 additions & 5 deletions wolfcrypt/src/asn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1877,7 +1877,7 @@ int GetASN_Items(const ASNItem* asn, ASNGetData *data, int count, int complete,
#endif

/* Set the end index at each depth to be the length. */
for (i=0; i<GET_ASN_MAX_DEPTH; i++) {
for (i = 0; i < GET_ASN_MAX_DEPTH; i++) {
endIdx[i] = length;
}

Expand All @@ -1889,14 +1889,19 @@ int GetASN_Items(const ASNItem* asn, ASNGetData *data, int count, int complete,
data[i].offset = idx;
/* Length of data in ASN.1 item starts empty. */
data[i].length = 0;
/* Get current item depth. */
depth = asn[i].depth;
if (depth >= GET_ASN_MAX_DEPTH) {
#ifdef WOLFSSL_DEBUG_ASN_TEMPLATE
WOLFSSL_MSG("Depth in template too large");
#endif
return ASN_PARSE_E;
}
/* Determine the current depth by checking index against end indices.
* Don't go lower than the expected depth. Depths lower than first
* may not have an end index set yet. */
while ((depth > asn[i].depth) &&
((depth <= asn[0].depth) || (idx == endIdx[depth]))) {
depth--;
}
/* Keep track of minimum depth. */
if (depth < minDepth) {
minDepth = depth;
Expand All @@ -1917,10 +1922,23 @@ int GetASN_Items(const ASNItem* asn, ASNGetData *data, int count, int complete,
}
}

/* Check for end of data or not a choice and tag not matching. */
/* A constructed item the data has not used up is not finished, so
* the template must not be moving out of it. */
if (depth > asn[i].depth) {
#ifdef WOLFSSL_DEBUG_ASN_TEMPLATE
WOLFSSL_MSG_VSNPRINTF("Depth %d in template, %d in data: %d",
asn[i].depth, depth, i);
#endif
return ASN_PARSE_E;
}

/* Check for data not reaching this depth, end of data, or not a choice
* and tag not matching. Data not this deep means the item's enclosing
* item was never entered and the item cannot be present. */
tmpW32Val = endIdx[depth];
XFENCE(); /* Prevent memory access */
if (idx == tmpW32Val || (data[i].dataType != ASN_DATA_TYPE_CHOICE &&
if ((depth < asn[i].depth) || idx == tmpW32Val ||
(data[i].dataType != ASN_DATA_TYPE_CHOICE &&
(input[idx] & ~ASN_CONSTRUCTED) != asn[i].tag)) {
if (asn[i].optional) {
/* Skip over ASN.1 items underneath this optional item. */
Expand Down Expand Up @@ -2088,6 +2106,9 @@ int GetASN_Items(const ASNItem* asn, ASNGetData *data, int count, int complete,
/* Store reference to data and length. */
data[i].data.ref.data = input + idx;
data[i].data.ref.length = (word32)len;
/* Index left at the start of the content - the items that
* follow are parsed out of this one, so move into it. */
depth++;
continue;
}

Expand Down
Loading
Loading