diff --git a/tests/api/test_pkcs12.c b/tests/api/test_pkcs12.c index 62730b0f03..74bb315b41 100644 --- a/tests/api/test_pkcs12.c +++ b/tests/api/test_pkcs12.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -272,6 +273,195 @@ int test_wc_d2i_PKCS12_oid_underflow(void) return EXPECT_RESULT(); } +/* Test that validates the fix for heap OOB read vulnerability where + * ASN.1 parsing after DecryptContent() would use stale ContentInfo bounds. + * This is a basic test that verifies the fix compiles and basic PKCS#12 + * functionality still works after adding contentSz bounds checking. */ +int test_wc_PKCS12_encrypted_content_bounds(void) +{ + EXPECT_DECLS; +#if !defined(NO_ASN) && !defined(NO_PWDBASED) && defined(HAVE_PKCS12) && \ + !defined(NO_RSA) && !defined(NO_AES) && !defined(NO_SHA) && \ + !defined(NO_SHA256) && defined(USE_CERT_BUFFERS_2048) + + /* This test validates that the fix for heap OOB read is in place. + * The fix ensures ASN.1 parsing uses contentSz (actual decrypted size) + * instead of ci->dataSz (original ContentInfo size) as bounds. + * + * We test this by exercising the PKCS#12 parsing path with encrypted + * content to ensure the fix doesn't break normal operation. */ + + byte* inKey = (byte*) server_key_der_2048; + const word32 inKeySz = sizeof_server_key_der_2048; + byte* inCert = (byte*) server_cert_der_2048; + const word32 inCertSz = sizeof_server_cert_der_2048; + WC_DerCertList inCa = { + (byte*)ca_cert_der_2048, sizeof_ca_cert_der_2048, NULL + }; + char pkcs12Passwd[] = "test_bounds_fix"; + + WC_PKCS12* pkcs12Export = NULL; + WC_PKCS12* pkcs12Import = NULL; + byte* pkcs12Der = NULL; + byte* outKey = NULL; + byte* outCert = NULL; + WC_DerCertList* outCaList = NULL; + int exportRet = 0; + word32 pkcs12DerSz = 0; + word32 outKeySz = 0; + word32 outCertSz = 0; + + /* Create a PKCS#12 with encrypted content */ + ExpectNotNull(pkcs12Export = wc_PKCS12_create(pkcs12Passwd, + sizeof(pkcs12Passwd) - 1, NULL, inKey, inKeySz, inCert, inCertSz, + &inCa, -1, -1, 2048, 2048, 0, NULL)); + + /* Serialize to DER - use int intermediate to avoid word32 truncation + * of negative error codes from wc_i2d_PKCS12(). */ + ExpectIntGE((exportRet = wc_i2d_PKCS12(pkcs12Export, &pkcs12Der, NULL)), 0); + pkcs12DerSz = (word32)exportRet; + + /* Parse it back - this exercises the fixed bounds checking code path */ + ExpectNotNull(pkcs12Import = wc_PKCS12_new_ex(NULL)); + ExpectIntGE(wc_d2i_PKCS12(pkcs12Der, pkcs12DerSz, pkcs12Import), 0); + + /* This parse operation now uses contentSz instead of ci->dataSz for bounds, + * preventing the heap OOB read that existed before the fix */ + ExpectIntEQ(wc_PKCS12_parse(pkcs12Import, pkcs12Passwd, &outKey, &outKeySz, + &outCert, &outCertSz, &outCaList), 0); + + /* Verify the parsing worked correctly */ + ExpectIntEQ(outKeySz, inKeySz); + ExpectIntEQ(outCertSz, inCertSz); + ExpectNotNull(outCaList); + ExpectIntEQ(outCaList->bufferSz, inCa.bufferSz); + + /* Clean up */ + XFREE(outKey, NULL, DYNAMIC_TYPE_PUBLIC_KEY); + XFREE(outCert, NULL, DYNAMIC_TYPE_PKCS); + wc_FreeCertList(outCaList, NULL); + wc_PKCS12_free(pkcs12Import); + XFREE(pkcs12Der, NULL, DYNAMIC_TYPE_PKCS); + wc_PKCS12_free(pkcs12Export); + +#endif + + /* Part 2: True regression test - craft a malformed PKCS#12 whose decrypted + * SafeBags SEQUENCE claims a length that exceeds the decrypted content + * bounds (contentSz) but fits within the stale ContentInfo bounds + * (ci->dataSz). Before the fix, the parser used ci->dataSz, allowing a + * heap OOB read; with the fix it uses contentSz and rejects the blob. */ +#if !defined(NO_ASN) && !defined(NO_PWDBASED) && defined(HAVE_PKCS12) && \ + defined(WOLFSSL_AES_256) && defined(HAVE_AES_CBC) && \ + defined(HAVE_AES_DECRYPT) && !defined(NO_SHA256) && !defined(NO_HMAC) && \ + defined(WOLFSSL_ASN_TEMPLATE) && !defined(HAVE_FIPS) + { + static const char regPassword[] = "test"; + static const byte regSalt[8] = {1, 2, 3, 4, 5, 6, 7, 8}; + static const byte regIv[16] = {0}; + + /* Malformed SafeBags plaintext (one AES block = 16 bytes). + * The outer SEQUENCE claims length 100 - this exceeds the decrypted + * content size (16) but fits inside the stale ci->dataSz (127) that + * the unfixed code used as the parsing bound. */ + static const byte regPlaintext[16] = { + 0x30, 0x64, /* SEQUENCE, length 100 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + }; + + /* Complete PKCS#12 DER (170 bytes). + * Structure: PFX { version 3, authSafe { DATA { AuthenticatedSafe { + * EncryptedData { PBES2(AES-256-CBC, HMAC-SHA256, PBKDF2) + * } } } } } + * No MacData - macIter=0 skips MAC verification. */ + byte regDer[170] = { + 0x30, 0x81, 0xA7, /* PFX SEQ (167) */ + 0x02, 0x01, 0x03, /* version 3 */ + 0x30, 0x81, 0xA1, /* authSafe ContentInfo (161) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, + 0xF7, 0x0D, 0x01, 0x07, 0x01, /* OID data */ + 0xA0, 0x81, 0x93, /* [0] CONS. (147) */ + 0x04, 0x81, 0x90, /* OCTET STRING (144) */ + 0x30, 0x81, 0x8D, /* AuthenticatedSafe SEQ (141) */ + 0x30, 0x81, 0x8A, /* ContentInfo SEQ (138) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, + 0xF7, 0x0D, 0x01, 0x07, 0x06, /* OID encryptedData */ + 0xA0, 0x7D, /* [0] CONS. (125) */ + 0x30, 0x7B, /* EncryptedData SEQ (123) */ + 0x02, 0x01, 0x00, /* version 0 */ + 0x30, 0x76, /* EncryptedContentInfo SEQ (118) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, + 0xF7, 0x0D, 0x01, 0x07, 0x01, /* OID data */ + /* --- EncryptContent payload (107 bytes) --- */ + 0x30, 0x57, /* AlgorithmIdentifier SEQ (87) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, + 0xF7, 0x0D, 0x01, 0x05, 0x0D, /* OID pbes2 */ + 0x30, 0x4A, /* PBES2-params SEQ (74) */ + 0x30, 0x29, /* keyDerivFunc SEQ (41) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, + 0xF7, 0x0D, 0x01, 0x05, 0x0C, /* OID pbkdf2 */ + 0x30, 0x1C, /* PBKDF2-params SEQ (28) */ + 0x04, 0x08, + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, /* salt */ + 0x02, 0x02, 0x08, 0x00, /* iterations 2048 */ + 0x30, 0x0C, /* PRF SEQ (12) */ + 0x06, 0x08, 0x2A, 0x86, 0x48, 0x86, + 0xF7, 0x0D, 0x02, 0x09, /* OID hmac-sha256 */ + 0x05, 0x00, /* NULL */ + 0x30, 0x1D, /* encryptionScheme SEQ (29) */ + 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, + 0x65, 0x03, 0x04, 0x01, 0x2A, /* OID aes256-cbc */ + 0x04, 0x10, /* IV OCT (16) */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x80, 0x10, /* [0] IMPLICIT CT (16) */ + /* 16 bytes ciphertext - filled at runtime */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + }; + + byte regKey[32]; + byte regCiphertext[16]; + Aes regAes; + WC_PKCS12* regP12 = NULL; + byte* regPkey = NULL; + byte* regCert = NULL; + word32 regPkeySz = 0; + word32 regCertSz = 0; + + /* Derive AES-256 key with the same PBKDF2 that DecryptContent uses */ + ExpectIntEQ(wc_PBKDF2(regKey, (const byte*)regPassword, + (int)XSTRLEN(regPassword), regSalt, (int)sizeof(regSalt), + 2048, 32, WC_SHA256), 0); + + /* Encrypt the malformed plaintext */ + ExpectIntEQ(wc_AesInit(®Aes, NULL, INVALID_DEVID), 0); + ExpectIntEQ(wc_AesSetKey(®Aes, regKey, 32, regIv, + AES_ENCRYPTION), 0); + ExpectIntEQ(wc_AesCbcEncrypt(®Aes, regCiphertext, regPlaintext, + sizeof(regPlaintext)), 0); + wc_AesFree(®Aes); + + /* Patch ciphertext into the DER template at offset 154 */ + XMEMCPY(regDer + 154, regCiphertext, sizeof(regCiphertext)); + + /* Parse the crafted PKCS#12 - d2i should succeed (outer structure + * is valid), but wc_PKCS12_parse must fail because GetSequence + * rejects SEQUENCE length 100 against contentSz 16. */ + ExpectNotNull(regP12 = wc_PKCS12_new_ex(NULL)); + ExpectIntGE(wc_d2i_PKCS12(regDer, (word32)sizeof(regDer), regP12), 0); + ExpectIntLT(wc_PKCS12_parse(regP12, regPassword, ®Pkey, ®PkeySz, + ®Cert, ®CertSz, NULL), 0); + + XFREE(regPkey, NULL, DYNAMIC_TYPE_PUBLIC_KEY); + XFREE(regCert, NULL, DYNAMIC_TYPE_PKCS); + wc_PKCS12_free(regP12); + } +#endif + return EXPECT_RESULT(); +} + int test_wc_PKCS12_PBKDF(void) { EXPECT_DECLS; @@ -675,4 +865,3 @@ int test_wc_PKCS12_PBKDF_ex_sha512_256(void) #endif return EXPECT_RESULT(); } - diff --git a/tests/api/test_pkcs12.h b/tests/api/test_pkcs12.h index 23acd2d3c7..0c2314d57d 100644 --- a/tests/api/test_pkcs12.h +++ b/tests/api/test_pkcs12.h @@ -28,6 +28,7 @@ int test_wc_i2d_PKCS12(void); int test_wc_PKCS12_create(void); int test_wc_d2i_PKCS12_bad_mac_salt(void); int test_wc_d2i_PKCS12_oid_underflow(void); +int test_wc_PKCS12_encrypted_content_bounds(void); int test_wc_PKCS12_PBKDF(void); int test_wc_PKCS12_PBKDF_ex(void); int test_wc_PKCS12_PBKDF_ex_sha1(void); @@ -42,6 +43,7 @@ int test_wc_PKCS12_PBKDF_ex_sha512_256(void); TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_create), \ TEST_DECL_GROUP("pkcs12", test_wc_d2i_PKCS12_bad_mac_salt), \ TEST_DECL_GROUP("pkcs12", test_wc_d2i_PKCS12_oid_underflow), \ + TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_encrypted_content_bounds), \ TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_PBKDF), \ TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_PBKDF_ex), \ TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_PBKDF_ex_sha1), \ diff --git a/wolfcrypt/src/pkcs12.c b/wolfcrypt/src/pkcs12.c index f3f5197833..39cddfc9ce 100644 --- a/wolfcrypt/src/pkcs12.c +++ b/wolfcrypt/src/pkcs12.c @@ -1335,6 +1335,7 @@ int wc_PKCS12_parse_ex(WC_PKCS12* pkcs12, const char* psw, byte* buf = NULL; word32 i, oid; word32 algId; + word32 contentSz; int ret, pswSz; #ifdef ASN_BER_TO_DER int curIdx; @@ -1450,6 +1451,11 @@ int wc_PKCS12_parse_ex(WC_PKCS12* pkcs12, const char* psw, goto exit_pk12par; } + /* DecryptContent strips the PBE ASN.1 wrapper and returns the + * actual decrypted payload size, which is smaller than the + * allocated buf. Track the real bounds so subsequent ASN.1 + * parsing does not read past the decrypted content. */ + contentSz = (word32)ret; data = buf; idx = 0; @@ -1486,36 +1492,39 @@ int wc_PKCS12_parse_ex(WC_PKCS12* pkcs12, const char* psw, goto exit_pk12par; } + /* DATA branch: data still points into ci->data, so the + * ContentInfo size is the correct parsing bound. */ + contentSz = ci->dataSz; } /* parse through bags in ContentInfo */ - if ((ret = GetSequence(data, &idx, &totalSz, ci->dataSz)) < 0) { + if ((ret = GetSequence(data, &idx, &totalSz, contentSz)) < 0) { goto exit_pk12par; } totalSz += (int)idx; while ((int)idx < totalSz) { int bagSz; - if ((ret = GetSequence(data, &idx, &bagSz, ci->dataSz)) < 0) { + if ((ret = GetSequence(data, &idx, &bagSz, contentSz)) < 0) { goto exit_pk12par; } bagSz += (int)idx; if ((ret = GetObjectId(data, &idx, &oid, oidIgnoreType, - ci->dataSz)) < 0) { + contentSz)) < 0) { goto exit_pk12par; } switch (oid) { case WC_PKCS12_KeyBag: /* 667 */ WOLFSSL_MSG("PKCS12 Key Bag found"); - if (GetASNTag(data, &idx, &tag, ci->dataSz) < 0) { + if (GetASNTag(data, &idx, &tag, contentSz) < 0) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if (tag != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC)) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } - if ((ret = GetLength(data, &idx, &size, ci->dataSz)) <= 0) { + if ((ret = GetLength(data, &idx, &size, contentSz)) <= 0) { if (ret == 0) ret = ASN_PARSE_E; goto exit_pk12par; @@ -1553,14 +1562,14 @@ int wc_PKCS12_parse_ex(WC_PKCS12* pkcs12, const char* psw, byte* k; WOLFSSL_MSG("PKCS12 Shrouded Key Bag found"); - if (GetASNTag(data, &idx, &tag, ci->dataSz) < 0) { + if (GetASNTag(data, &idx, &tag, contentSz) < 0) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if (tag != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC)) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if ((ret = GetLength(data, &idx, &size, - ci->dataSz)) < 0) { + contentSz)) < 0) { goto exit_pk12par; } @@ -1626,23 +1635,23 @@ int wc_PKCS12_parse_ex(WC_PKCS12* pkcs12, const char* psw, { WC_DerCertList* node; WOLFSSL_MSG("PKCS12 Cert Bag found"); - if (GetASNTag(data, &idx, &tag, ci->dataSz) < 0) { + if (GetASNTag(data, &idx, &tag, contentSz) < 0) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if (tag != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC)) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } - if ((ret = GetLength(data, &idx, &size, ci->dataSz)) < 0) { + if ((ret = GetLength(data, &idx, &size, contentSz)) < 0) { goto exit_pk12par; } /* get cert bag type */ - if ((ret = GetSequence(data, &idx, &size, ci->dataSz)) <0) { + if ((ret = GetSequence(data, &idx, &size, contentSz)) <0) { goto exit_pk12par; } if ((ret = GetObjectId(data, &idx, &oid, oidIgnoreType, - ci->dataSz)) < 0) { + contentSz)) < 0) { goto exit_pk12par; } @@ -1650,27 +1659,27 @@ int wc_PKCS12_parse_ex(WC_PKCS12* pkcs12, const char* psw, case WC_PKCS12_CertBag_Type1: /* 675 */ /* type 1 */ WOLFSSL_MSG("PKCS12 cert bag type 1"); - if (GetASNTag(data, &idx, &tag, ci->dataSz) < 0) { + if (GetASNTag(data, &idx, &tag, contentSz) < 0) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if (tag != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC)) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } - if ((ret = GetLength(data, &idx, &size, ci->dataSz)) + if ((ret = GetLength(data, &idx, &size, contentSz)) <= 0) { if (ret == 0) ret = ASN_PARSE_E; goto exit_pk12par; } - if (GetASNTag(data, &idx, &tag, ci->dataSz) < 0) { + if (GetASNTag(data, &idx, &tag, contentSz) < 0) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if (tag != ASN_OCTET_STRING) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } - if ((ret = GetLength(data, &idx, &size, ci->dataSz)) + if ((ret = GetLength(data, &idx, &size, contentSz)) < 0) { goto exit_pk12par; }