Skip to content

Commit 4c6c744

Browse files
FrauschiFrauschi
committed
Fix various range and size bugs in PKCS#7 code
1 parent a50a540 commit 4c6c744

2 files changed

Lines changed: 159 additions & 7 deletions

File tree

‎tests/api.c‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34994,6 +34994,107 @@ static int test_pkcs7_ori_oversized_oid(void)
3499434994
return EXPECT_RESULT();
3499534995
}
3499634996

34997+
/* ORI callback that flags if oriValueSz looks like an underflow (>= 0x80000000) */
34998+
#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC)
34999+
static int test_ori_underflow_cb(wc_PKCS7* pkcs7, byte* oriType,
35000+
word32 oriTypeSz, byte* oriValue,
35001+
word32 oriValueSz, byte* decryptedKey,
35002+
word32* decryptedKeySz, void* ctx)
35003+
{
35004+
int* called = (int*)ctx;
35005+
(void)pkcs7; (void)oriType; (void)oriTypeSz;
35006+
(void)oriValue; (void)decryptedKey; (void)decryptedKeySz;
35007+
if (called != NULL)
35008+
*called = (int)oriValueSz; /* record what we received */
35009+
return -1;
35010+
}
35011+
#endif
35012+
35013+
/* Test: PKCS#7 ORI must reject when OID consumption exceeds the [4] implicit
35014+
* SEQUENCE length (integer underflow in oriValueSz computation).
35015+
*
35016+
* With implicit tagging, [4] CONSTRUCTED replaces the SEQUENCE tag, so
35017+
* wc_PKCS7_DecryptOri reads seqSz directly from the [4] length field.
35018+
* We set [4] length = 5 while the OID inside consumes 22 bytes
35019+
* (tag + length + 20 content), triggering oriValueSz = 5 - 22 = underflow.
35020+
*
35021+
* The buffer includes a dummy EncryptedContentInfo after the RecipientInfos
35022+
* so the total message is large enough for the PKCS7 streaming code (which
35023+
* requests the full remaining message before parsing the ORI). */
35024+
static int test_pkcs7_ori_seqsz_underflow(void)
35025+
{
35026+
EXPECT_DECLS;
35027+
#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC)
35028+
wc_PKCS7* p7 = NULL;
35029+
byte out[256];
35030+
int cbCalled = 0;
35031+
35032+
/*
35033+
* Byte layout (all outer lengths match actual byte counts on wire):
35034+
*
35035+
* OID inside [4]: 06 14 <20 bytes> = 22 bytes
35036+
* [4] (declared len 5, actual content 22):
35037+
* a4 05 <22 bytes> = 24 bytes on wire
35038+
* SET: 31 18 <24 bytes> = 26 bytes on wire
35039+
* version: 02 01 00 = 3 bytes
35040+
* EncryptedContentInfo (filler, never parsed):
35041+
* 30 0b { 06 09 <9 bytes OID> } = 13 bytes on wire
35042+
* EnvelopedData: 30 2a <3+26+13=42 bytes> = 44 bytes on wire
35043+
* [0] EXPLICIT: a0 2c <44 bytes> = 46 bytes on wire
35044+
* OID(envelopedData): 06 09 <9 bytes> = 11 bytes on wire
35045+
* ContentInfo: 30 39 <11+46=57 bytes> = 59 bytes total
35046+
*/
35047+
static const byte poc[] = {
35048+
/* ContentInfo SEQUENCE (length 57) */
35049+
0x30, 0x39,
35050+
/* contentType = envelopedData 1.2.840.113549.1.7.3 */
35051+
0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03,
35052+
/* [0] EXPLICIT (length 44) */
35053+
0xa0, 0x2c,
35054+
/* EnvelopedData SEQUENCE (length 42) */
35055+
0x30, 0x2a,
35056+
/* version = 0 */
35057+
0x02, 0x01, 0x00,
35058+
/* RecipientInfos SET (length 24) */
35059+
0x31, 0x18,
35060+
/* [4] CONSTRUCTED = ORI implicit SEQUENCE, declared len 5 */
35061+
/* Actual OID is 22 bytes → exceeds declared 5 */
35062+
0xa4, 0x05,
35063+
/* OID: tag=06, len=0x14(20), content=20 bytes = 22 total */
35064+
0x06, 0x14,
35065+
0x2a, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
35066+
0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11,
35067+
0x12, 0x13, 0x14, 0x15,
35068+
/* EncryptedContentInfo SEQUENCE (length 11) — filler so
35069+
* streaming has enough data; never actually parsed because
35070+
* DecryptOri fails before we get here */
35071+
0x30, 0x0b,
35072+
/* contentType = data 1.2.840.113549.1.7.1 */
35073+
0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07,
35074+
0x01
35075+
};
35076+
35077+
p7 = wc_PKCS7_New(NULL, INVALID_DEVID);
35078+
ExpectNotNull(p7);
35079+
if (p7 != NULL) {
35080+
wc_PKCS7_SetOriDecryptCb(p7, test_ori_underflow_cb);
35081+
wc_PKCS7_SetOriDecryptCtx(p7, &cbCalled);
35082+
35083+
/* Must return an error before the callback sees an underflowed size */
35084+
ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc),
35085+
out, sizeof(out)), 0);
35086+
35087+
/* The callback must NOT have been invoked with a wrapped oriValueSz.
35088+
* cbCalled == 0 means the callback was never reached (ideal).
35089+
* cbCalled < 0 would indicate the underflow was passed through. */
35090+
ExpectIntGE(cbCalled, 0);
35091+
35092+
wc_PKCS7_Free(p7);
35093+
}
35094+
#endif
35095+
return EXPECT_RESULT();
35096+
}
35097+
3499735098
/* Dilithium verify_ctx_msg must reject absurdly large msgLen */
3499835099
static int test_dilithium_hash(void)
3499935100
{
@@ -35939,6 +36040,7 @@ TEST_CASE testCases[] = {
3593936040
TEST_DECL(test_ed448_rejects_identity_key),
3594036041
TEST_DECL(test_pkcs7_decode_encrypted_outputsz),
3594136042
TEST_DECL(test_pkcs7_ori_oversized_oid),
36043+
TEST_DECL(test_pkcs7_ori_seqsz_underflow),
3594236044
TEST_DECL(test_pkcs7_padding),
3594336045

3594436046
#if defined(WOLFSSL_SNIFFER) && defined(WOLFSSL_SNIFFER_CHAIN_INPUT)

‎wolfcrypt/src/pkcs7.c‎

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10758,15 +10758,19 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1075810758
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1075910759
return ASN_PARSE_E;
1076010760

10761-
if ((word32)keyIdSize > pkiMsgSz - (*idx))
10761+
/* Validate SKID container and keyIdSize against buffer */
10762+
if ((word32)length > pkiMsgSz - (*idx))
1076210763
return BUFFER_E;
1076310764

10765+
if (length < keyIdSize)
10766+
return ASN_PARSE_E;
10767+
1076410768
/* if we found correct recipient, SKID will match */
1076510769
if (XMEMCMP(pkiMsg + (*idx), pkcs7->issuerSubjKeyId,
1076610770
(word32)keyIdSize) == 0) {
1076710771
*recipFound = 1;
1076810772
}
10769-
(*idx) += (word32)keyIdSize;
10773+
(*idx) += (word32)length;
1077010774
}
1077110775

1077210776
if (GetAlgoId(pkiMsg, idx, &encOID, oidKeyType, pkiMsgSz) < 0)
@@ -11043,6 +11047,14 @@ static int wc_PKCS7_KariGetOriginatorIdentifierOrKey(WC_PKCS7_KARI* kari,
1104311047
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1104411048
return ASN_PARSE_E;
1104511049

11050+
/* BIT STRING must have at least unused-bits byte + 1 byte of content */
11051+
if (length < 2)
11052+
return ASN_PARSE_E;
11053+
11054+
/* Validate BIT STRING content is within input buffer */
11055+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx)
11056+
return ASN_PARSE_E;
11057+
1104611058
if (GetASNTag(pkiMsg, idx, &tag, pkiMsgSz) < 0)
1104711059
return ASN_EXPECT_0_E;
1104811060

@@ -11522,9 +11534,22 @@ static int wc_PKCS7_DecryptOri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1152211534
XMEMCPY(oriOID, pkiMsg + *idx, (word32)oriOIDSz);
1152311535
*idx += (word32)oriOIDSz;
1152411536

11537+
/* Validate OID did not consume more than the SEQUENCE declared */
11538+
if ((*idx - tmpIdx) > (word32)seqSz) {
11539+
WOLFSSL_MSG("ORI oriType OID exceeds SEQUENCE boundary");
11540+
return ASN_PARSE_E;
11541+
}
11542+
1152511543
/* get oriValue, increment idx */
1152611544
oriValue = pkiMsg + *idx;
1152711545
oriValueSz = (word32)seqSz - (*idx - tmpIdx);
11546+
11547+
/* Validate oriValue region is within input buffer */
11548+
if (*idx > pkiMsgSz || oriValueSz > pkiMsgSz - *idx) {
11549+
WOLFSSL_MSG("ORI oriValue exceeds input buffer");
11550+
return ASN_PARSE_E;
11551+
}
11552+
1152811553
*idx += oriValueSz;
1152911554

1153011555
/* pass oriOID and oriValue to user callback, expect back
@@ -11702,6 +11727,12 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1170211727
return ASN_PARSE_E;
1170311728
}
1170411729

11730+
/* Validate IV is within input buffer */
11731+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) {
11732+
XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
11733+
return ASN_PARSE_E;
11734+
}
11735+
1170511736
XMEMCPY(tmpIv, pkiMsg + (*idx), (word32)length);
1170611737
*idx += (word32)length;
1170711738

@@ -11721,6 +11752,12 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1172111752
return ASN_PARSE_E;
1172211753
}
1172311754

11755+
/* Validate EncryptedKey is within input buffer */
11756+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) {
11757+
XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
11758+
return ASN_PARSE_E;
11759+
}
11760+
1172411761
/* allocate temporary space for decrypted key */
1172511762
cekSz = (word32)length;
1172611763
cek = (byte*)XMALLOC(cekSz, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
@@ -11807,7 +11844,7 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1180711844
byte* keyId = NULL;
1180811845
const byte* datePtr = NULL;
1180911846
byte dateFormat, tag;
11810-
word32 keyIdSz, kekIdSz, keyWrapOID, localIdx;
11847+
word32 keyIdSz, kekIdSz, kekIdEnd, keyWrapOID, localIdx;
1181111848

1181211849
int ret = 0;
1181311850
byte* pkiMsg = in;
@@ -11833,6 +11870,11 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1183311870
return ASN_PARSE_E;
1183411871

1183511872
kekIdSz = (word32)length;
11873+
kekIdEnd = *idx + kekIdSz;
11874+
11875+
/* Validate KEKIdentifier boundary is within input buffer */
11876+
if (kekIdEnd < *idx || kekIdEnd > pkiMsgSz)
11877+
return ASN_PARSE_E;
1183611878

1183711879
if (GetASNTag(pkiMsg, idx, &tag, pkiMsgSz) < 0)
1183811880
return ASN_PARSE_E;
@@ -11843,17 +11885,21 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1184311885
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1184411886
return ASN_PARSE_E;
1184511887

11888+
/* Validate keyIdentifier is within input buffer */
11889+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx)
11890+
return ASN_PARSE_E;
11891+
1184611892
/* save keyIdentifier and length */
1184711893
keyId = pkiMsg + *idx;
1184811894
keyIdSz = (word32)length;
1184911895
*idx += keyIdSz;
1185011896

1185111897
/* may have OPTIONAL GeneralizedTime */
1185211898
localIdx = *idx;
11853-
if ((*idx < kekIdSz) && GetASNTag(pkiMsg, &localIdx, &tag,
11899+
if ((*idx < kekIdEnd) && GetASNTag(pkiMsg, &localIdx, &tag,
1185411900
pkiMsgSz) == 0 && tag == ASN_GENERALIZED_TIME) {
11855-
if (wc_GetDateInfo(pkiMsg + *idx, (int)pkiMsgSz, &datePtr,
11856-
&dateFormat, &dateLen) != 0) {
11901+
if (wc_GetDateInfo(pkiMsg + *idx, (int)(pkiMsgSz - *idx),
11902+
&datePtr, &dateFormat, &dateLen) != 0) {
1185711903
return ASN_PARSE_E;
1185811904
}
1185911905
*idx += (word32)(dateLen + 1);
@@ -11865,7 +11911,7 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1186511911

1186611912
/* may have OPTIONAL OtherKeyAttribute */
1186711913
localIdx = *idx;
11868-
if ((*idx < kekIdSz) && GetASNTag(pkiMsg, &localIdx, &tag,
11914+
if ((*idx < kekIdEnd) && GetASNTag(pkiMsg, &localIdx, &tag,
1186911915
pkiMsgSz) == 0 && tag == (ASN_SEQUENCE |
1187011916
ASN_CONSTRUCTED)) {
1187111917
if (GetSequence(pkiMsg, idx, &length, pkiMsgSz) < 0)
@@ -11894,6 +11940,10 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1189411940
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1189511941
return ASN_PARSE_E;
1189611942

11943+
/* Validate EncryptedKey is within input buffer */
11944+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx)
11945+
return ASN_PARSE_E;
11946+
1189711947
#ifndef NO_AES
1189811948
direction = AES_DECRYPTION;
1189911949
#else

0 commit comments

Comments
 (0)