Skip to content

Commit f6a1857

Browse files
padelsbachpadelsbach
committed
Fix OOB possibility in PKCS7_VerifySignedData
1 parent 3351eb4 commit f6a1857

3 files changed

Lines changed: 133 additions & 1 deletion

File tree

‎tests/api/test_pkcs7.c‎

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

‎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)