Skip to content

Fix pkcs12 parse issue #10172

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
embhorn wants to merge 5 commits into
base: master
Choose a base branch
from
+216 −16
Open

Fix pkcs12 parse issue #10172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fc9484
Fix pkcs12 parse issue
embhorn Apr 8, 2026
1e54e99
Fix word32 truncation and add true regression test for PKCS12 OOB read
embhorn Apr 9, 2026
edc1643
Fix from review
embhorn Apr 9, 2026
3bf4f7e
Fix from review
embhorn Apr 9, 2026
7d485df
Fix from review
embhorn Apr 9, 2026
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
191 changes: 190 additions & 1 deletion tests/api/test_pkcs12.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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)
* <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;
Expand Down Expand Up @@ -675,4 +865,3 @@ int test_wc_PKCS12_PBKDF_ex_sha512_256(void)
#endif
return EXPECT_RESULT();
}

2 changes: 2 additions & 0 deletions tests/api/test_pkcs12.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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), \
Expand Down
39 changes: 24 additions & 15 deletions wolfcrypt/src/pkcs12.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1626,51 +1635,51 @@ 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;
}

switch (oid) {
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;
}
Expand Down
Loading