Skip to content

Commit c6fe7cb

Browse files
padelsbachpadelsbach
committed
Fix OOB possibility in PKCS7_VerifySignedData
1 parent e98fb8f commit c6fe7cb

3 files changed

Lines changed: 132 additions & 1 deletion

File tree

‎tests/api/test_pkcs7.c‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5312,3 +5312,110 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void)
53125312
#endif /* HAVE_PKCS7 && !NO_PKCS7_STREAM */
53135313
return EXPECT_RESULT();
53145314
}
5315+
5316+
/*
5317+
* SignedData bundle truncated at the eContent [0] EXPLICIT tag in
5318+
* encapContentInfo. Verifies that the parser rejects the malformed
5319+
* input rather than dereferencing past the end of the buffer.
5320+
*/
5321+
int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void)
5322+
{
5323+
EXPECT_DECLS;
5324+
#if defined(HAVE_PKCS7)
5325+
PKCS7* pkcs7 = NULL;
5326+
5327+
WOLFSSL_SMALL_STACK_STATIC byte der[] = {
5328+
/* outer ContentInfo SEQUENCE (75 bytes content) */
5329+
0x30, 0x4B,
5330+
/* contentType OID 1.2.840.113549.1.7.2 (signedData) */
5331+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02,
5332+
/* [0] EXPLICIT (62 bytes content) */
5333+
0xA0, 0x3E,
5334+
/* SignedData SEQUENCE (60 bytes content) */
5335+
0x30, 0x3C,
5336+
/* version INTEGER 1 */
5337+
0x02, 0x01, 0x01,
5338+
/* digestAlgorithms SET (empty - degenerate) */
5339+
0x31, 0x00,
5340+
/* encapContentInfo SEQUENCE (53 bytes content) */
5341+
0x30, 0x35,
5342+
/* eContentType OID with 50 bytes of arbitrary payload */
5343+
0x06, 0x32,
5344+
0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01, 0x00,
5345+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5346+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5347+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5348+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5349+
/* eContent [0] EXPLICIT — buffer ends here, no length, no content */
5350+
0xA0
5351+
};
5352+
word32 derSz = (word32)sizeof(der);
5353+
5354+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
5355+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
5356+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
5357+
ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0);
5358+
wc_PKCS7_Free(pkcs7);
5359+
5360+
#endif /* HAVE_PKCS7 */
5361+
return EXPECT_RESULT();
5362+
}
5363+
5364+
/*
5365+
* SignedData bundle truncated at the certificates [0] IMPLICIT tag.
5366+
* Verifies that the parser rejects the malformed input rather than
5367+
* dereferencing past the end of the buffer.
5368+
*
5369+
* TODO: limited to NO_PKCS7_STREAM because the streaming parser's stage 3
5370+
* early-exit check (pkcs7.c near line 6594) accepts any bundle
5371+
* whose remaining footer is < 6 bytes as a successful degenerate end,
5372+
* so the bounds check at line 6765 is unreachable in streaming mode.
5373+
* Drop the NO_PKCS7_STREAM gate if/when the early-exit is resolved.
5374+
*/
5375+
int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void)
5376+
{
5377+
EXPECT_DECLS;
5378+
#if defined(HAVE_PKCS7) && defined(NO_PKCS7_STREAM)
5379+
PKCS7* pkcs7 = NULL;
5380+
5381+
WOLFSSL_SMALL_STACK_STATIC byte der[] = {
5382+
/* outer ContentInfo SEQUENCE (78 bytes content) */
5383+
0x30, 0x4E,
5384+
/* contentType OID signedData */
5385+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02,
5386+
/* [0] EXPLICIT (65 bytes content) */
5387+
0xA0, 0x41,
5388+
/* SignedData SEQUENCE (63 bytes content) */
5389+
0x30, 0x3F,
5390+
/* version INTEGER 1 */
5391+
0x02, 0x01, 0x01,
5392+
/* digestAlgorithms SET (empty) */
5393+
0x31, 0x00,
5394+
/* encapContentInfo SEQUENCE (55 bytes content) */
5395+
0x30, 0x37,
5396+
/* eContentType OID 1.2.840.113549.1.7.1 (data) */
5397+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01,
5398+
/* eContent [0] EXPLICIT (42 bytes content) */
5399+
0xA0, 0x2A,
5400+
/* OCTET STRING (40 bytes content) */
5401+
0x04, 0x28,
5402+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5403+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5404+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5405+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5406+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
5407+
/* certificates [0] IMPLICIT — buffer ends here, no length */
5408+
0xA0
5409+
};
5410+
word32 derSz = (word32)sizeof(der);
5411+
5412+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
5413+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
5414+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
5415+
ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0);
5416+
wc_PKCS7_Free(pkcs7);
5417+
5418+
#endif /* HAVE_PKCS7 && NO_PKCS7_STREAM */
5419+
return EXPECT_RESULT();
5420+
}
5421+

‎tests/api/test_pkcs7.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void);
6868
int test_wc_PKCS7_DecodeEnvelopedData_forgedRecipientSetLen(void);
6969
int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void);
7070
int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
71+
int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void);
72+
int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void);
7173

7274

7375
#define TEST_PKCS7_DECLS \
@@ -115,7 +117,9 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
115117
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_BER), \
116118
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_NoDefaultSignedAttribs), \
117119
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq), \
118-
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_IndefLenOOB)
120+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_IndefLenOOB), \
121+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncEContentTag), \
122+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncCertSetTag)
119123

120124
#define TEST_PKCS7_ENCRYPTED_DATA_DECLS \
121125
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_stream), \

‎wolfcrypt/src/pkcs7.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6333,6 +6333,14 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
63336333
* OCTET_STRING will be next. If so, we use the length retrieved
63346334
* there. PKCS#7 spec defines ANY as eContent type. In this case
63356335
* we fall back and save this content length for use later */
6336+
if (ret == 0 && localIdx >= pkiMsgSz) {
6337+
/* Truncated input: don't dereference past the buffer.
6338+
* Break out of the switch directly so the degenerate-
6339+
* recovery path below cannot mask this error. */
6340+
ret = BUFFER_E;
6341+
break;
6342+
}
6343+
63366344
if (ret == 0 && pkiMsg[localIdx] != ASN_INDEF_LENGTH) {
63376345
if (GetLength_ex(pkiMsg, &localIdx, &length, pkiMsgSz,
63386346
NO_USER_CHECK) <= 0) {
@@ -6590,6 +6598,14 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
65906598

65916599
/* check if bundle has more elements or footer, if not, set content
65926600
* to pkcs7->content and hash to pkcs7->hash.
6601+
*
6602+
* NOTE: this check returns success whenever fewer than 6 bytes
6603+
* follow the content within the outer ContentInfo, which also
6604+
* accepts truncated bundles whose footer was cut short (e.g. a
6605+
* lone certificates [0] tag with no length). Distinguishing a
6606+
* legitimate degenerate end (such as an empty signerInfos SET
6607+
* "31 00") from truncated junk would require peeking at the
6608+
* remaining bytes or making stage 4's `expected` window smaller.
65936609
*/
65946610
if (ret == 0 && pkcs7->stream->maxLen > 0 &&
65956611
(pkcs7->stream->maxLen - pkcs7->stream->totalRd)
@@ -6759,6 +6775,10 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
67596775
&& tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) {
67606776
idx++;
67616777

6778+
if (localIdx >= pkiMsg2Sz) {
6779+
ret = BUFFER_E;
6780+
}
6781+
67626782
/* if certificates set has indefinite length, try to get
67636783
* the first certificate length of the set.
67646784
*/

0 commit comments

Comments
 (0)