mirror of
https://github.com/wolfSSL/wolfssl.git
synced 2026-07-05 12:20:52 +02:00
Fix pkcs12 parse issue
This commit is contained in:
@@ -272,6 +272,78 @@ 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;
|
||||
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 */
|
||||
ExpectIntGE((pkcs12DerSz = wc_i2d_PKCS12(pkcs12Export, &pkcs12Der, NULL)), 0);
|
||||
|
||||
/* 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
|
||||
return EXPECT_RESULT();
|
||||
}
|
||||
|
||||
int test_wc_PKCS12_PBKDF(void)
|
||||
{
|
||||
EXPECT_DECLS;
|
||||
|
||||
@@ -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), \
|
||||
|
||||
+24
-15
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user