Skip to content

Commit 84fb0f6

Browse files
FrauschiFrauschi
committed
Fix various range and size bugs in PKCS#7 code
1 parent b573823 commit 84fb0f6

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
@@ -35859,6 +35859,107 @@ static int test_pkcs7_ori_oversized_oid(void)
3585935859
return EXPECT_RESULT();
3586035860
}
3586135861

35862+
/* ORI callback that flags if oriValueSz looks like an underflow (>= 0x80000000) */
35863+
#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC)
35864+
static int test_ori_underflow_cb(wc_PKCS7* pkcs7, byte* oriType,
35865+
word32 oriTypeSz, byte* oriValue,
35866+
word32 oriValueSz, byte* decryptedKey,
35867+
word32* decryptedKeySz, void* ctx)
35868+
{
35869+
int* called = (int*)ctx;
35870+
(void)pkcs7; (void)oriType; (void)oriTypeSz;
35871+
(void)oriValue; (void)decryptedKey; (void)decryptedKeySz;
35872+
if (called != NULL)
35873+
*called = (int)oriValueSz; /* record what we received */
35874+
return -1;
35875+
}
35876+
#endif
35877+
35878+
/* Test: PKCS#7 ORI must reject when OID consumption exceeds the [4] implicit
35879+
* SEQUENCE length (integer underflow in oriValueSz computation).
35880+
*
35881+
* With implicit tagging, [4] CONSTRUCTED replaces the SEQUENCE tag, so
35882+
* wc_PKCS7_DecryptOri reads seqSz directly from the [4] length field.
35883+
* We set [4] length = 5 while the OID inside consumes 22 bytes
35884+
* (tag + length + 20 content), triggering oriValueSz = 5 - 22 = underflow.
35885+
*
35886+
* The buffer includes a dummy EncryptedContentInfo after the RecipientInfos
35887+
* so the total message is large enough for the PKCS7 streaming code (which
35888+
* requests the full remaining message before parsing the ORI). */
35889+
static int test_pkcs7_ori_seqsz_underflow(void)
35890+
{
35891+
EXPECT_DECLS;
35892+
#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC)
35893+
wc_PKCS7* p7 = NULL;
35894+
byte out[256];
35895+
int cbCalled = 0;
35896+
35897+
/*
35898+
* Byte layout (all outer lengths match actual byte counts on wire):
35899+
*
35900+
* OID inside [4]: 06 14 <20 bytes> = 22 bytes
35901+
* [4] (declared len 5, actual content 22):
35902+
* a4 05 <22 bytes> = 24 bytes on wire
35903+
* SET: 31 18 <24 bytes> = 26 bytes on wire
35904+
* version: 02 01 00 = 3 bytes
35905+
* EncryptedContentInfo (filler, never parsed):
35906+
* 30 0b { 06 09 <9 bytes OID> } = 13 bytes on wire
35907+
* EnvelopedData: 30 2a <3+26+13=42 bytes> = 44 bytes on wire
35908+
* [0] EXPLICIT: a0 2c <44 bytes> = 46 bytes on wire
35909+
* OID(envelopedData): 06 09 <9 bytes> = 11 bytes on wire
35910+
* ContentInfo: 30 39 <11+46=57 bytes> = 59 bytes total
35911+
*/
35912+
static const byte poc[] = {
35913+
/* ContentInfo SEQUENCE (length 57) */
35914+
0x30, 0x39,
35915+
/* contentType = envelopedData 1.2.840.113549.1.7.3 */
35916+
0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03,
35917+
/* [0] EXPLICIT (length 44) */
35918+
0xa0, 0x2c,
35919+
/* EnvelopedData SEQUENCE (length 42) */
35920+
0x30, 0x2a,
35921+
/* version = 0 */
35922+
0x02, 0x01, 0x00,
35923+
/* RecipientInfos SET (length 24) */
35924+
0x31, 0x18,
35925+
/* [4] CONSTRUCTED = ORI implicit SEQUENCE, declared len 5 */
35926+
/* Actual OID is 22 bytes -> exceeds declared 5 */
35927+
0xa4, 0x05,
35928+
/* OID: tag=06, len=0x14(20), content=20 bytes = 22 total */
35929+
0x06, 0x14,
35930+
0x2a, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
35931+
0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11,
35932+
0x12, 0x13, 0x14, 0x15,
35933+
/* EncryptedContentInfo SEQUENCE (length 11) - filler so
35934+
* streaming has enough data; never actually parsed because
35935+
* DecryptOri fails before we get here */
35936+
0x30, 0x0b,
35937+
/* contentType = data 1.2.840.113549.1.7.1 */
35938+
0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07,
35939+
0x01
35940+
};
35941+
35942+
p7 = wc_PKCS7_New(NULL, INVALID_DEVID);
35943+
ExpectNotNull(p7);
35944+
if (p7 != NULL) {
35945+
wc_PKCS7_SetOriDecryptCb(p7, test_ori_underflow_cb);
35946+
wc_PKCS7_SetOriDecryptCtx(p7, &cbCalled);
35947+
35948+
/* Must return an error before the callback sees an underflowed size */
35949+
ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc),
35950+
out, sizeof(out)), 0);
35951+
35952+
/* The callback must NOT have been invoked with a wrapped oriValueSz.
35953+
* cbCalled == 0 means the callback was never reached (ideal).
35954+
* cbCalled < 0 would indicate the underflow was passed through. */
35955+
ExpectIntGE(cbCalled, 0);
35956+
35957+
wc_PKCS7_Free(p7);
35958+
}
35959+
#endif
35960+
return EXPECT_RESULT();
35961+
}
35962+
3586235963
/* Dilithium verify_ctx_msg must reject absurdly large msgLen */
3586335964
static int test_dilithium_hash(void)
3586435965
{
@@ -36800,6 +36901,7 @@ TEST_CASE testCases[] = {
3680036901
TEST_DECL(test_ed448_rejects_identity_key),
3680136902
TEST_DECL(test_pkcs7_decode_encrypted_outputsz),
3680236903
TEST_DECL(test_pkcs7_ori_oversized_oid),
36904+
TEST_DECL(test_pkcs7_ori_seqsz_underflow),
3680336905
TEST_DECL(test_pkcs7_padding),
3680436906

3680536907
#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
@@ -10769,15 +10769,19 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1076910769
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1077010770
return ASN_PARSE_E;
1077110771

10772-
if ((word32)keyIdSize > pkiMsgSz - (*idx))
10772+
/* Validate SKID container and keyIdSize against buffer */
10773+
if ((word32)length > pkiMsgSz - (*idx))
1077310774
return BUFFER_E;
1077410775

10776+
if (length < keyIdSize)
10777+
return ASN_PARSE_E;
10778+
1077510779
/* if we found correct recipient, SKID will match */
1077610780
if (XMEMCMP(pkiMsg + (*idx), pkcs7->issuerSubjKeyId,
1077710781
(word32)keyIdSize) == 0) {
1077810782
*recipFound = 1;
1077910783
}
10780-
(*idx) += (word32)keyIdSize;
10784+
(*idx) += (word32)length;
1078110785
}
1078210786

1078310787
if (GetAlgoId(pkiMsg, idx, &encOID, oidKeyType, pkiMsgSz) < 0)
@@ -11054,6 +11058,14 @@ static int wc_PKCS7_KariGetOriginatorIdentifierOrKey(WC_PKCS7_KARI* kari,
1105411058
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1105511059
return ASN_PARSE_E;
1105611060

11061+
/* BIT STRING must have at least unused-bits byte + 1 byte of content */
11062+
if (length < 2)
11063+
return ASN_PARSE_E;
11064+
11065+
/* Validate BIT STRING content is within input buffer */
11066+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx)
11067+
return ASN_PARSE_E;
11068+
1105711069
if (GetASNTag(pkiMsg, idx, &tag, pkiMsgSz) < 0)
1105811070
return ASN_EXPECT_0_E;
1105911071

@@ -11533,9 +11545,22 @@ static int wc_PKCS7_DecryptOri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1153311545
XMEMCPY(oriOID, pkiMsg + *idx, (word32)oriOIDSz);
1153411546
*idx += (word32)oriOIDSz;
1153511547

11548+
/* Validate OID did not consume more than the SEQUENCE declared */
11549+
if ((*idx - tmpIdx) > (word32)seqSz) {
11550+
WOLFSSL_MSG("ORI oriType OID exceeds SEQUENCE boundary");
11551+
return ASN_PARSE_E;
11552+
}
11553+
1153611554
/* get oriValue, increment idx */
1153711555
oriValue = pkiMsg + *idx;
1153811556
oriValueSz = (word32)seqSz - (*idx - tmpIdx);
11557+
11558+
/* Validate oriValue region is within input buffer */
11559+
if (*idx > pkiMsgSz || oriValueSz > pkiMsgSz - *idx) {
11560+
WOLFSSL_MSG("ORI oriValue exceeds input buffer");
11561+
return ASN_PARSE_E;
11562+
}
11563+
1153911564
*idx += oriValueSz;
1154011565

1154111566
/* pass oriOID and oriValue to user callback, expect back
@@ -11713,6 +11738,12 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1171311738
return ASN_PARSE_E;
1171411739
}
1171511740

11741+
/* Validate IV is within input buffer */
11742+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) {
11743+
XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
11744+
return ASN_PARSE_E;
11745+
}
11746+
1171611747
XMEMCPY(tmpIv, pkiMsg + (*idx), (word32)length);
1171711748
*idx += (word32)length;
1171811749

@@ -11732,6 +11763,12 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1173211763
return ASN_PARSE_E;
1173311764
}
1173411765

11766+
/* Validate EncryptedKey is within input buffer */
11767+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) {
11768+
XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
11769+
return ASN_PARSE_E;
11770+
}
11771+
1173511772
/* allocate temporary space for decrypted key */
1173611773
cekSz = (word32)length;
1173711774
cek = (byte*)XMALLOC(cekSz, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
@@ -11818,7 +11855,7 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1181811855
byte* keyId = NULL;
1181911856
const byte* datePtr = NULL;
1182011857
byte dateFormat, tag;
11821-
word32 keyIdSz, kekIdSz, keyWrapOID, localIdx;
11858+
word32 keyIdSz, kekIdSz, kekIdEnd, keyWrapOID, localIdx;
1182211859

1182311860
int ret = 0;
1182411861
byte* pkiMsg = in;
@@ -11844,6 +11881,11 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1184411881
return ASN_PARSE_E;
1184511882

1184611883
kekIdSz = (word32)length;
11884+
kekIdEnd = *idx + kekIdSz;
11885+
11886+
/* Validate KEKIdentifier boundary is within input buffer */
11887+
if (kekIdEnd < *idx || kekIdEnd > pkiMsgSz)
11888+
return ASN_PARSE_E;
1184711889

1184811890
if (GetASNTag(pkiMsg, idx, &tag, pkiMsgSz) < 0)
1184911891
return ASN_PARSE_E;
@@ -11854,17 +11896,21 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1185411896
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1185511897
return ASN_PARSE_E;
1185611898

11899+
/* Validate keyIdentifier is within input buffer */
11900+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx)
11901+
return ASN_PARSE_E;
11902+
1185711903
/* save keyIdentifier and length */
1185811904
keyId = pkiMsg + *idx;
1185911905
keyIdSz = (word32)length;
1186011906
*idx += keyIdSz;
1186111907

1186211908
/* may have OPTIONAL GeneralizedTime */
1186311909
localIdx = *idx;
11864-
if ((*idx < kekIdSz) && GetASNTag(pkiMsg, &localIdx, &tag,
11910+
if ((*idx < kekIdEnd) && GetASNTag(pkiMsg, &localIdx, &tag,
1186511911
pkiMsgSz) == 0 && tag == ASN_GENERALIZED_TIME) {
11866-
if (wc_GetDateInfo(pkiMsg + *idx, (int)pkiMsgSz, &datePtr,
11867-
&dateFormat, &dateLen) != 0) {
11912+
if (wc_GetDateInfo(pkiMsg + *idx, (int)(pkiMsgSz - *idx),
11913+
&datePtr, &dateFormat, &dateLen) != 0) {
1186811914
return ASN_PARSE_E;
1186911915
}
1187011916
*idx += (word32)(dateLen + 1);
@@ -11876,7 +11922,7 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1187611922

1187711923
/* may have OPTIONAL OtherKeyAttribute */
1187811924
localIdx = *idx;
11879-
if ((*idx < kekIdSz) && GetASNTag(pkiMsg, &localIdx, &tag,
11925+
if ((*idx < kekIdEnd) && GetASNTag(pkiMsg, &localIdx, &tag,
1188011926
pkiMsgSz) == 0 && tag == (ASN_SEQUENCE |
1188111927
ASN_CONSTRUCTED)) {
1188211928
if (GetSequence(pkiMsg, idx, &length, pkiMsgSz) < 0)
@@ -11905,6 +11951,10 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz,
1190511951
if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0)
1190611952
return ASN_PARSE_E;
1190711953

11954+
/* Validate EncryptedKey is within input buffer */
11955+
if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx)
11956+
return ASN_PARSE_E;
11957+
1190811958
#ifndef NO_AES
1190911959
direction = AES_DECRYPTION;
1191011960
#else

0 commit comments

Comments
 (0)