Skip to content

Commit 1e54e99

Browse files
embhornembhorn
committed
Fix word32 truncation and add true regression test for PKCS12 OOB read
1 parent 0fc9484 commit 1e54e99

1 file changed

Lines changed: 120 additions & 2 deletions

File tree

‎tests/api/test_pkcs12.c‎

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <wolfssl/wolfcrypt/pkcs12.h>
3232
#include <wolfssl/wolfcrypt/pwdbased.h>
3333
#include <wolfssl/wolfcrypt/types.h>
34+
#include <wolfssl/wolfcrypt/aes.h>
3435
#include <tests/api/api.h>
3536
#include <tests/api/test_pkcs12.h>
3637

@@ -305,6 +306,7 @@ int test_wc_PKCS12_encrypted_content_bounds(void)
305306
byte* outKey = NULL;
306307
byte* outCert = NULL;
307308
WC_DerCertList* outCaList = NULL;
309+
int exportRet = 0;
308310
word32 pkcs12DerSz = 0;
309311
word32 outKeySz = 0;
310312
word32 outCertSz = 0;
@@ -314,8 +316,10 @@ int test_wc_PKCS12_encrypted_content_bounds(void)
314316
sizeof(pkcs12Passwd) - 1, NULL, inKey, inKeySz, inCert, inCertSz,
315317
&inCa, -1, -1, 2048, 2048, 0, NULL));
316318

317-
/* Serialize to DER */
318-
ExpectIntGE((pkcs12DerSz = wc_i2d_PKCS12(pkcs12Export, &pkcs12Der, NULL)), 0);
319+
/* Serialize to DER — use int intermediate to avoid word32 truncation
320+
* of negative error codes from wc_i2d_PKCS12(). */
321+
ExpectIntGE((exportRet = wc_i2d_PKCS12(pkcs12Export, &pkcs12Der, NULL)), 0);
322+
pkcs12DerSz = (word32)exportRet;
319323

320324
/* Parse it back - this exercises the fixed bounds checking code path */
321325
ExpectNotNull(pkcs12Import = wc_PKCS12_new_ex(NULL));
@@ -340,6 +344,120 @@ int test_wc_PKCS12_encrypted_content_bounds(void)
340344
XFREE(pkcs12Der, NULL, DYNAMIC_TYPE_PKCS);
341345
wc_PKCS12_free(pkcs12Export);
342346

347+
#endif
348+
349+
/* Part 2: True regression test — craft a malformed PKCS#12 whose decrypted
350+
* SafeBags SEQUENCE claims a length that exceeds the decrypted content
351+
* bounds (contentSz) but fits within the stale ContentInfo bounds
352+
* (ci->dataSz). Before the fix, the parser used ci->dataSz, allowing a
353+
* heap OOB read; with the fix it uses contentSz and rejects the blob. */
354+
#if !defined(NO_ASN) && !defined(NO_PWDBASED) && defined(HAVE_PKCS12) && \
355+
defined(WOLFSSL_AES_256) && defined(HAVE_AES_CBC) && \
356+
defined(HAVE_AES_DECRYPT) && !defined(NO_SHA256) && !defined(NO_HMAC) && \
357+
defined(WOLFSSL_ASN_TEMPLATE)
358+
{
359+
static const char regPassword[] = "test";
360+
static const byte regSalt[8] = {1, 2, 3, 4, 5, 6, 7, 8};
361+
static const byte regIv[16] = {0};
362+
363+
/* Malformed SafeBags plaintext (one AES block = 16 bytes).
364+
* The outer SEQUENCE claims length 100 — this exceeds the decrypted
365+
* content size (16) but fits inside the stale ci->dataSz (127) that
366+
* the unfixed code used as the parsing bound. */
367+
static const byte regPlaintext[16] = {
368+
0x30, 0x64, /* SEQUENCE, length 100 */
369+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
370+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00
371+
};
372+
373+
/* Complete PKCS#12 DER (170 bytes).
374+
* Structure: PFX { version 3, authSafe { DATA { AuthenticatedSafe {
375+
* EncryptedData { PBES2(AES-256-CBC, HMAC-SHA256, PBKDF2)
376+
* <ciphertext placeholder at offset 154> } } } } }
377+
* No MacData — macIter=0 skips MAC verification. */
378+
byte regDer[170] = {
379+
0x30, 0x81, 0xA7, /* PFX SEQ (167) */
380+
0x02, 0x01, 0x03, /* version 3 */
381+
0x30, 0x81, 0xA1, /* authSafe ContentInfo (161) */
382+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
383+
0xF7, 0x0D, 0x01, 0x07, 0x01, /* OID data */
384+
0xA0, 0x81, 0x93, /* [0] CONS. (147) */
385+
0x04, 0x81, 0x90, /* OCTET STRING (144) */
386+
0x30, 0x81, 0x8D, /* AuthenticatedSafe SEQ (141) */
387+
0x30, 0x81, 0x8A, /* ContentInfo SEQ (138) */
388+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
389+
0xF7, 0x0D, 0x01, 0x07, 0x06, /* OID encryptedData */
390+
0xA0, 0x7D, /* [0] CONS. (125) */
391+
0x30, 0x7B, /* EncryptedData SEQ (123) */
392+
0x02, 0x01, 0x00, /* version 0 */
393+
0x30, 0x76, /* EncryptedContentInfo SEQ (118) */
394+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
395+
0xF7, 0x0D, 0x01, 0x07, 0x01, /* OID data */
396+
/* --- EncryptContent payload (107 bytes) --- */
397+
0x30, 0x57, /* AlgorithmIdentifier SEQ (87) */
398+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
399+
0xF7, 0x0D, 0x01, 0x05, 0x0D, /* OID pbes2 */
400+
0x30, 0x4A, /* PBES2-params SEQ (74) */
401+
0x30, 0x29, /* keyDerivFunc SEQ (41) */
402+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
403+
0xF7, 0x0D, 0x01, 0x05, 0x0C, /* OID pbkdf2 */
404+
0x30, 0x1C, /* PBKDF2-params SEQ (28) */
405+
0x04, 0x08,
406+
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, /* salt */
407+
0x02, 0x02, 0x08, 0x00, /* iterations 2048 */
408+
0x30, 0x0C, /* PRF SEQ (12) */
409+
0x06, 0x08, 0x2A, 0x86, 0x48, 0x86,
410+
0xF7, 0x0D, 0x02, 0x09, /* OID hmac-sha256 */
411+
0x05, 0x00, /* NULL */
412+
0x30, 0x1D, /* encryptionScheme SEQ (29) */
413+
0x06, 0x09, 0x60, 0x86, 0x48, 0x01,
414+
0x65, 0x03, 0x04, 0x01, 0x2A, /* OID aes256-cbc */
415+
0x04, 0x10, /* IV OCT (16) */
416+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
417+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
418+
0x80, 0x10, /* [0] IMPLICIT CT (16) */
419+
/* 16 bytes ciphertext — filled at runtime */
420+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
421+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
422+
};
423+
424+
byte regKey[32];
425+
byte regCiphertext[16];
426+
Aes* regAes = NULL;
427+
WC_PKCS12* regP12 = NULL;
428+
byte* regPkey = NULL;
429+
byte* regCert = NULL;
430+
word32 regPkeySz = 0;
431+
word32 regCertSz = 0;
432+
433+
/* Derive AES-256 key with the same PBKDF2 that DecryptContent uses */
434+
ExpectIntEQ(wc_PBKDF2(regKey, (const byte*)regPassword,
435+
(int)XSTRLEN(regPassword), regSalt, (int)sizeof(regSalt),
436+
2048, 32, WC_SHA256), 0);
437+
438+
/* Encrypt the malformed plaintext */
439+
ExpectNotNull(regAes = wc_AesNew(NULL, INVALID_DEVID, NULL));
440+
ExpectIntEQ(wc_AesSetKey(regAes, regKey, 32, regIv,
441+
AES_ENCRYPTION), 0);
442+
ExpectIntEQ(wc_AesCbcEncrypt(regAes, regCiphertext, regPlaintext,
443+
sizeof(regPlaintext)), 0);
444+
wc_AesFree(regAes);
445+
446+
/* Patch ciphertext into the DER template at offset 154 */
447+
XMEMCPY(regDer + 154, regCiphertext, sizeof(regCiphertext));
448+
449+
/* Parse the crafted PKCS#12 — d2i should succeed (outer structure
450+
* is valid), but wc_PKCS12_parse must fail because GetSequence
451+
* rejects SEQUENCE length 100 against contentSz 16. */
452+
ExpectNotNull(regP12 = wc_PKCS12_new_ex(NULL));
453+
ExpectIntGE(wc_d2i_PKCS12(regDer, (word32)sizeof(regDer), regP12), 0);
454+
ExpectIntLT(wc_PKCS12_parse(regP12, regPassword, &regPkey, &regPkeySz,
455+
&regCert, &regCertSz, NULL), 0);
456+
457+
XFREE(regPkey, NULL, DYNAMIC_TYPE_PUBLIC_KEY);
458+
XFREE(regCert, NULL, DYNAMIC_TYPE_PKCS);
459+
wc_PKCS12_free(regP12);
460+
}
343461
#endif
344462
return EXPECT_RESULT();
345463
}

0 commit comments

Comments
 (0)