Harden PKCS#7 FlattenEncodedAttribs

This commit is contained in:
Eric Blankenhorn
2026-06-08 09:16:43 -05:00
parent 8fca95ce65
commit 9f7c536e23
3 changed files with 289 additions and 14 deletions
+209
View File
@@ -3873,6 +3873,215 @@ int test_wc_PKCS7_EncodeEncryptedData(void)
} /* END test_wc_PKCS7_EncodeEncryptedData() */
/*
* Regression test for an integer overflow in the PKCS#7 attribute encode
* path. An application-supplied PKCS7Attrib.valueSz close to UINT32_MAX used
* to wrap the word32 size accumulation in EncodeAttributes() /
* FlattenEncodedAttribs(), yielding an undersized allocation followed by a
* multi-gigabyte XMEMCPY (heap buffer overflow). The encode call must now
* reject the oversized attribute with an error rather than overflow.
*/
int test_wc_PKCS7_EncodeEncryptedData_AttribOverflow(void)
{
EXPECT_DECLS;
#if defined(HAVE_PKCS7) && !defined(NO_PKCS7_ENCRYPTED_DATA) && ( \
(!defined(NO_AES) && defined(HAVE_AES_CBC) && \
(defined(WOLFSSL_AES_256) || defined(WOLFSSL_AES_128))) || \
!defined(NO_DES3))
PKCS7* pkcs7 = NULL;
byte output[TWOK_BUF];
PKCS7Attrib attrib;
/* Small, valid attribute buffers. The encode path must reject the
* oversized valueSz before ever dereferencing attrib.value. */
static const byte oid[] = { 0x06, 0x03, 0x55, 0x04, 0x03 };
static const byte value[] = { 0x04, 0x01, 0x00 };
const byte data[] = { /* Hello World */
0x48,0x65,0x6c,0x6c,0x6f,0x20,0x57,0x6f,0x72,0x6c,0x64
};
#if !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_256)
byte key[] = {
0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,
0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,
0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,
0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08
};
int encryptOID = AES256CBCb;
#elif !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_128)
byte key[] = {
0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,
0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08
};
int encryptOID = AES128CBCb;
#else
byte key[] = {
0x01,0x23,0x45,0x67,0x89,0xab,0xcd,0xef,
0xfe,0xde,0xba,0x98,0x76,0x54,0x32,0x10,
0x89,0xab,0xcd,0xef,0x01,0x23,0x45,0x67
};
int encryptOID = DES3b;
#endif
XMEMSET(&attrib, 0, sizeof(attrib));
attrib.oid = oid;
attrib.oidSz = (word32)sizeof(oid);
attrib.value = value;
/* word32 wraparound trigger: valueSz + encoded header sizes overflows */
attrib.valueSz = 0xFFFFFFF4U;
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, testDevId), 0);
if (pkcs7 != NULL) {
pkcs7->content = (byte*)data;
pkcs7->contentSz = (word32)sizeof(data);
pkcs7->contentOID = DATA;
pkcs7->encryptOID = encryptOID;
pkcs7->encryptionKey = key;
pkcs7->encryptionKeySz = (word32)sizeof(key);
pkcs7->unprotectedAttribs = &attrib;
pkcs7->unprotectedAttribsSz = 1;
pkcs7->heap = HEAP_HINT;
}
ExpectIntEQ(wc_PKCS7_EncodeEncryptedData(pkcs7, output, sizeof(output)),
WC_NO_ERR_TRACE(BUFFER_E));
wc_PKCS7_Free(pkcs7);
#endif
return EXPECT_RESULT();
} /* END test_wc_PKCS7_EncodeEncryptedData_AttribOverflow() */
/*
* Same overflow guard, exercised through the SignedData attribute path
* (pkcs7->signedAttribs -> wc_PKCS7_BuildSignedAttributes -> EncodeAttributes).
* The encode must reject the oversized attribute instead of overflowing.
*/
int test_wc_PKCS7_EncodeSignedData_AttribOverflow(void)
{
EXPECT_DECLS;
#if defined(HAVE_PKCS7) && defined(USE_CERT_BUFFERS_2048) && !defined(NO_RSA)
PKCS7* pkcs7 = NULL;
WC_RNG rng;
byte output[FOURK_BUF];
byte data[] = "Test data to encode.";
PKCS7Attrib attrib;
static const byte oid[] = { 0x06, 0x03, 0x55, 0x04, 0x03 };
static const byte value[] = { 0x04, 0x01, 0x00 };
XMEMSET(&rng, 0, sizeof(WC_RNG));
XMEMSET(&attrib, 0, sizeof(attrib));
attrib.oid = oid;
attrib.oidSz = (word32)sizeof(oid);
attrib.value = value;
/* word32 wraparound trigger */
attrib.valueSz = 0xFFFFFFF4U;
ExpectIntEQ(wc_InitRng(&rng), 0);
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048,
(word32)sizeof(client_cert_der_2048)), 0);
if (pkcs7 != NULL) {
pkcs7->content = data;
pkcs7->contentSz = (word32)sizeof(data);
pkcs7->privateKey = (byte*)client_key_der_2048;
pkcs7->privateKeySz = (word32)sizeof(client_key_der_2048);
pkcs7->encryptOID = RSAk;
#if defined(NO_SHA) || defined(WC_FIPS_186_5_PLUS)
pkcs7->hashOID = SHA256h;
#else
pkcs7->hashOID = SHAh;
#endif
pkcs7->rng = &rng;
pkcs7->signedAttribs = &attrib;
pkcs7->signedAttribsSz = 1;
}
ExpectIntEQ(wc_PKCS7_EncodeSignedData(pkcs7, output, sizeof(output)),
WC_NO_ERR_TRACE(BUFFER_E));
wc_PKCS7_Free(pkcs7);
DoExpectIntEQ(wc_FreeRng(&rng), 0);
#endif
return EXPECT_RESULT();
} /* END test_wc_PKCS7_EncodeSignedData_AttribOverflow() */
/*
* Same overflow guard, exercised through the AuthEnvelopedData attribute
* paths. Case 1 covers a malicious authenticated attribute; case 2 supplies a
* valid authenticated attribute (which forces allocation of the auth attrib
* and AAD buffers) together with a malicious unauthenticated attribute, so the
* more complex unauth cleanup path (FreeEncodedRecipientSet + XFREE(aadBuffer)
* + XFREE(flatAuthAttribs)) is exercised. Both must return an error.
*/
int test_wc_PKCS7_EncodeAuthEnvelopedData_AttribOverflow(void)
{
EXPECT_DECLS;
#if defined(HAVE_PKCS7) && defined(USE_CERT_BUFFERS_2048) && !defined(NO_RSA) && \
defined(HAVE_AESGCM) && !defined(NO_AES) && defined(WOLFSSL_AES_256)
PKCS7* pkcs7 = NULL;
byte output[FOURK_BUF];
byte data[] = "Test data to encode.";
PKCS7Attrib bad;
PKCS7Attrib good;
static const byte oid[] = { 0x06, 0x03, 0x55, 0x04, 0x03 };
static const byte value[] = { 0x04, 0x01, 0x00 };
XMEMSET(&bad, 0, sizeof(bad));
bad.oid = oid;
bad.oidSz = (word32)sizeof(oid);
bad.value = value;
/* word32 wraparound trigger */
bad.valueSz = 0xFFFFFFF4U;
XMEMSET(&good, 0, sizeof(good));
good.oid = oid;
good.oidSz = (word32)sizeof(oid);
good.value = value;
good.valueSz = (word32)sizeof(value);
/* Case 1: malicious authenticated attribute. */
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048,
(word32)sizeof(client_cert_der_2048)), 0);
if (pkcs7 != NULL) {
pkcs7->content = data;
pkcs7->contentSz = (word32)sizeof(data);
pkcs7->contentOID = DATA;
pkcs7->encryptOID = AES256GCMb;
pkcs7->authAttribs = &bad;
pkcs7->authAttribsSz = 1;
}
ExpectIntEQ(wc_PKCS7_EncodeAuthEnvelopedData(pkcs7, output, sizeof(output)),
WC_NO_ERR_TRACE(BUFFER_E));
wc_PKCS7_Free(pkcs7);
pkcs7 = NULL;
/* Case 2: valid authenticated attribute + malicious unauthenticated one. */
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048,
(word32)sizeof(client_cert_der_2048)), 0);
if (pkcs7 != NULL) {
pkcs7->content = data;
pkcs7->contentSz = (word32)sizeof(data);
pkcs7->contentOID = DATA;
pkcs7->encryptOID = AES256GCMb;
pkcs7->authAttribs = &good;
pkcs7->authAttribsSz = 1;
pkcs7->unauthAttribs = &bad;
pkcs7->unauthAttribsSz = 1;
}
ExpectIntEQ(wc_PKCS7_EncodeAuthEnvelopedData(pkcs7, output, sizeof(output)),
WC_NO_ERR_TRACE(BUFFER_E));
wc_PKCS7_Free(pkcs7);
#endif
return EXPECT_RESULT();
} /* END test_wc_PKCS7_EncodeAuthEnvelopedData_AttribOverflow() */
#if defined(HAVE_PKCS7) && defined(USE_CERT_BUFFERS_2048) && !defined(NO_DES3) && !defined(NO_RSA) && !defined(NO_SHA)
static void build_test_EncryptedKeyPackage(byte * out, word32 * out_size, byte * in_data, word32 in_size, size_t in_content_type, size_t test_vector)
{
+6
View File
@@ -29,6 +29,8 @@ int test_wc_PKCS7_Init(void);
int test_wc_PKCS7_InitWithCert(void);
int test_wc_PKCS7_EncodeData(void);
int test_wc_PKCS7_EncodeSignedData(void);
int test_wc_PKCS7_EncodeSignedData_AttribOverflow(void);
int test_wc_PKCS7_EncodeAuthEnvelopedData_AttribOverflow(void);
#if defined(HAVE_PKCS7) && defined(WC_RSA_PSS) && !defined(NO_RSA) && \
!defined(NO_FILESYSTEM) && !defined(NO_SHA256)
int test_wc_PKCS7_EncodeSignedData_RSA_PSS(void);
@@ -56,6 +58,7 @@ int test_wc_PKCS7_EncodeDecodeEnvelopedData(void);
int test_wc_PKCS7_SetAESKeyWrapUnwrapCb(void);
int test_wc_PKCS7_GetEnvelopedDataKariRid(void);
int test_wc_PKCS7_EncodeEncryptedData(void);
int test_wc_PKCS7_EncodeEncryptedData_AttribOverflow(void);
int test_wc_PKCS7_DecodeEncryptedKeyPackage(void);
int test_wc_PKCS7_DecodeSymmetricKeyPackage(void);
int test_wc_PKCS7_DecodeOneSymmetricKey(void);
@@ -111,6 +114,7 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void);
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_InitWithCert), \
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_EncodeData), \
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_EncodeSignedData), \
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_EncodeSignedData_AttribOverflow), \
TEST_PKCS7_RSA_PSS_SD_DECL \
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_EncodeSignedData_ex), \
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_RSA), \
@@ -133,6 +137,8 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void);
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_SetAESKeyWrapUnwrapCb), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_GetEnvelopedDataKariRid), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_EncodeEncryptedData), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_EncodeEncryptedData_AttribOverflow), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_EncodeAuthEnvelopedData_AttribOverflow), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEncryptedKeyPackage), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeSymmetricKeyPackage), \
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeOneSymmetricKey), \
+74 -14
View File
@@ -1641,9 +1641,25 @@ static int EncodeAttributes(EncodedAttrib* ea, int eaSz,
for (i = 0; i < maxSz; i++)
{
word32 attribSz = 0;
word32 boundSz = 0;
ea[i].value = attribs[i].value;
ea[i].valueSz = attribs[i].valueSz;
/* The valueSz and oidSz fields are application supplied and unbounded.
* Reject any attribute whose encoded size would overflow a word32
* before performing the size arithmetic below. Otherwise a large
* valueSz wraps the running total, producing an undersized allocation
* and a heap buffer overflow in FlattenEncodedAttribs(). The encoded
* SET and SEQUENCE headers add at most MAX_SET_SZ and MAX_SEQ_SZ bytes,
* so checking against those upper bounds bounds the real total. */
if (!WC_SAFE_SUM_WORD32(ea[i].valueSz, attribs[i].oidSz, boundSz) ||
!WC_SAFE_SUM_WORD32(boundSz, (word32)MAX_SET_SZ, boundSz) ||
!WC_SAFE_SUM_WORD32(boundSz, (word32)MAX_SEQ_SZ, boundSz)) {
WOLFSSL_MSG("PKCS7 attribute size overflow");
return BUFFER_E;
}
attribSz += ea[i].valueSz;
ea[i].valueSetSz = SetSet(attribSz, ea[i].valueSet);
attribSz += ea[i].valueSetSz;
@@ -1654,6 +1670,13 @@ static int EncodeAttributes(EncodedAttrib* ea, int eaSz,
attribSz += ea[i].valueSeqSz;
ea[i].totalSz = attribSz;
/* Keep the running total within positive int range so callers can
* distinguish a valid size (>= 0) from a negative error return. */
if (attribSz > (WOLFSSL_MAX_32BIT >> 1) ||
(word32)allAttribsSz > (WOLFSSL_MAX_32BIT >> 1) - attribSz) {
WOLFSSL_MSG("PKCS7 attributes total size overflow");
return BUFFER_E;
}
allAttribsSz += (int)attribSz;
}
return allAttribsSz;
@@ -1759,7 +1782,16 @@ static int FlattenEncodedAttribs(wc_PKCS7* pkcs7, FlatAttrib** derArr, int rows,
}
for (i = 0; i < eaSz; i++) {
sz = ea[i].valueSeqSz + ea[i].oidSz + ea[i].valueSetSz + ea[i].valueSz;
/* Defense in depth: guard the size sum against word32 overflow so the
* allocation below can never be smaller than the XMEMCPY lengths.
* EncodeAttributes() rejects oversized attributes up front, but this
* keeps the allocation safe if reached with unchecked input. */
if (!WC_SAFE_SUM_WORD32(ea[i].valueSeqSz, ea[i].oidSz, sz) ||
!WC_SAFE_SUM_WORD32(sz, ea[i].valueSetSz, sz) ||
!WC_SAFE_SUM_WORD32(sz, ea[i].valueSz, sz)) {
WOLFSSL_MSG("PKCS7 attribute size overflow");
return BUFFER_E;
}
output = (byte*)XMALLOC(sz, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
if (output == NULL) {
@@ -2218,6 +2250,7 @@ static int wc_PKCS7_BuildSignedAttributes(wc_PKCS7* pkcs7, ESD* esd,
byte* signingTime, word32 signingTimeSz)
{
int hashSz;
int encAttribsSz;
#ifdef NO_ASN_TIME
PKCS7Attrib cannedAttribs[2];
#else
@@ -2281,9 +2314,11 @@ static int wc_PKCS7_BuildSignedAttributes(wc_PKCS7* pkcs7, ESD* esd,
}
esd->signedAttribsCount += idx;
esd->signedAttribsSz += (word32)EncodeAttributes(
&esd->signedAttribs[atrIdx], (int)idx, cannedAttribs,
(int)idx);
encAttribsSz = EncodeAttributes(&esd->signedAttribs[atrIdx],
(int)idx, cannedAttribs, (int)idx);
if (encAttribsSz < 0)
return encAttribsSz;
esd->signedAttribsSz += (word32)encAttribsSz;
atrIdx += idx;
} else {
esd->signedAttribsCount = 0;
@@ -2298,9 +2333,12 @@ static int wc_PKCS7_BuildSignedAttributes(wc_PKCS7* pkcs7, ESD* esd,
return BUFFER_E;
esd->signedAttribsCount += pkcs7->signedAttribsSz;
esd->signedAttribsSz += (word32)EncodeAttributes(
&esd->signedAttribs[atrIdx], (int)esd->signedAttribsCount,
pkcs7->signedAttribs, (int)pkcs7->signedAttribsSz);
encAttribsSz = EncodeAttributes(&esd->signedAttribs[atrIdx],
(int)esd->signedAttribsCount, pkcs7->signedAttribs,
(int)pkcs7->signedAttribsSz);
if (encAttribsSz < 0)
return encAttribsSz;
esd->signedAttribsSz += (word32)encAttribsSz;
}
#ifdef NO_ASN_TIME
@@ -14335,18 +14373,26 @@ int wc_PKCS7_EncodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* output,
contentTypeAttrib.valueSz = pkcs7->contentTypeSz;
}
authAttribsSz += (word32)EncodeAttributes(authAttribs, 1,
&contentTypeAttrib, 1);
ret = EncodeAttributes(authAttribs, 1, &contentTypeAttrib, 1);
if (ret < 0) {
wc_PKCS7_FreeEncodedRecipientSet(pkcs7);
return ret;
}
authAttribsSz += (word32)ret;
authAttribsCount += 1;
}
/* authAttribs: add in user authenticated attributes */
if (pkcs7->authAttribs != NULL && pkcs7->authAttribsSz > 0) {
authAttribsSz += (word32)EncodeAttributes(
authAttribs + authAttribsCount,
ret = EncodeAttributes(authAttribs + authAttribsCount,
(int)(MAX_AUTH_ATTRIBS_SZ - authAttribsCount),
pkcs7->authAttribs,
(int)pkcs7->authAttribsSz);
if (ret < 0) {
wc_PKCS7_FreeEncodedRecipientSet(pkcs7);
return ret;
}
authAttribsSz += (word32)ret;
authAttribsCount += pkcs7->authAttribsSz;
}
@@ -14394,11 +14440,17 @@ int wc_PKCS7_EncodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* output,
/* build up unauthenticated attributes (unauthAttrs) */
if (pkcs7->unauthAttribsSz > 0) {
unauthAttribsSz = (word32)EncodeAttributes(
unauthAttribs + unauthAttribsCount,
ret = EncodeAttributes(unauthAttribs + unauthAttribsCount,
(int)(MAX_UNAUTH_ATTRIBS_SZ - unauthAttribsCount),
pkcs7->unauthAttribs,
(int)pkcs7->unauthAttribsSz);
if (ret < 0) {
wc_PKCS7_FreeEncodedRecipientSet(pkcs7);
XFREE(aadBuffer, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
XFREE(flatAuthAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
return ret;
}
unauthAttribsSz = (word32)ret;
unauthAttribsCount = pkcs7->unauthAttribsSz;
if (unauthAttribsSz > 0) {
@@ -15577,10 +15629,18 @@ int wc_PKCS7_EncodeEncryptedData(wc_PKCS7* pkcs7, byte* output, word32 outputSz)
}
attribsCount = pkcs7->unprotectedAttribsSz;
attribsSz = (word32)EncodeAttributes(attribs,
ret = EncodeAttributes(attribs,
(int)pkcs7->unprotectedAttribsSz,
pkcs7->unprotectedAttribs,
(int)pkcs7->unprotectedAttribsSz);
if (ret < 0) {
XFREE(attribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
ForceZero(plain, (word32)encryptedOutSz);
XFREE(plain, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
return ret;
}
attribsSz = (word32)ret;
if (attribsSz > 0) {
flatAttribs = (byte*)XMALLOC(attribsSz, pkcs7->heap,