wolfSSL/wolfssl
1
0
Fork 1
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-07-05 09:20:55 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #10172 from embhorn/zd21568

Browse Source 
Fix pkcs12 parse issue
This commit is contained in:
Sean Parkinson
2026-04-15 09:00:12 +10:00
committed by GitHub
parent 14ebd3d649 863db50318
commit 409b5fcf38
3 changed files with 219 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/api/test_pkcs12.c
+193 -1
View File
@@ -31,6 +31,7 @@
#include <wolfssl/wolfcrypt/pkcs12.h>
#include <wolfssl/wolfcrypt/pwdbased.h>
#include <wolfssl/wolfcrypt/types.h>
#include <wolfssl/wolfcrypt/aes.h>
#include <tests/api/api.h>
#include <tests/api/test_pkcs12.h>
@@ -272,6 +273,198 @@ 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);
ExpectIntEQ(XMEMCMP(outKey, inKey, inKeySz), 0);
ExpectIntEQ(XMEMCMP(outCert, inCert, inCertSz), 0);
ExpectIntEQ(XMEMCMP(outCaList->buffer, inCa.buffer, inCa.bufferSz), 0);
/* 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)
* <ciphertext placeholder at offset 154> } } } } }
* 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(&regAes, NULL, INVALID_DEVID), 0);
ExpectIntEQ(wc_AesSetKey(&regAes, regKey, 32, regIv,
AES_ENCRYPTION), 0);
ExpectIntEQ(wc_AesCbcEncrypt(&regAes, regCiphertext, regPlaintext,
sizeof(regPlaintext)), 0);
wc_AesFree(&regAes);
/* 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, &regPkey, &regPkeySz,
&regCert, &regCertSz, 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 +868,3 @@ int test_wc_PKCS12_PBKDF_ex_sha512_256(void)
#endif
return EXPECT_RESULT();
}
tests/api/test_pkcs12.h
+2
View File
@@ -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), \
wolfcrypt/src/pkcs12.c
+24 -15
View File
@@ -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 = 0;
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;
}