From 548e2e3099f4d2d2866cc9de27d81ce05de51cf2 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Fri, 20 Mar 2026 14:19:00 -0500 Subject: [PATCH 1/2] Fix FillSigner to clear pubkeystored --- wolfcrypt/src/asn.c | 2 + wolfcrypt/test/test.c | 111 ++++++++++++++++++++++++++++++++++++++++ wolfssl/wolfcrypt/asn.h | 15 ++++-- 3 files changed, 123 insertions(+), 5 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 9b2faf19c5..84f1d12eea 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -26880,7 +26880,9 @@ int FillSigner(Signer* signer, DecodedCert* cert, int type, DerBuffer *der) signer->extKeyUsage = cert->extExtKeyUsage; signer->next = NULL; /* If Key Usage not set, all uses valid. */ cert->publicKey = 0; /* in case lock fails don't free here. */ + cert->pubKeyStored = 0; cert->subjectCN = 0; + cert->subjectCNStored = 0; #ifndef IGNORE_NAME_CONSTRAINTS cert->permittedNames = NULL; cert->excludedNames = NULL; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 80866209c5..d86b557cac 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -822,6 +822,7 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t scrypt_test(void); #if !defined(NO_ASN_TIME) && !defined(NO_RSA) && defined(WOLFSSL_TEST_CERT) && \ !defined(NO_FILESYSTEM) WOLFSSL_TEST_SUBROUTINE wc_test_ret_t cert_test(void); +static wc_test_ret_t fill_signer_twice_test(void); #endif #if defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_TEST_CERT) && \ !defined(NO_FILESYSTEM) && !defined(NO_RSA) && defined(WOLFSSL_GEN_CERT) @@ -2733,6 +2734,11 @@ options: [-s max_relative_stack_bytes] [-m max_relative_heap_memory_bytes]\n\ TEST_FAIL("CERT test failed!\n", ret); else TEST_PASS("CERT test passed!\n"); + + if ( (ret = fill_signer_twice_test()) != 0) + TEST_FAIL("FILL SIGNER test failed!\n", ret); + else + TEST_PASS("FILL SIGNER test passed!\n"); #endif #if defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_TEST_CERT) && \ @@ -22252,6 +22258,111 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t cert_test(void) } #endif /* WOLFSSL_TEST_CERT */ +#if !defined(NO_ASN_TIME) && !defined(NO_RSA) && defined(WOLFSSL_TEST_CERT) && \ + !defined(NO_FILESYSTEM) +/* Test that FillSigner clears pubKeyStored/subjectCNStored after transferring + * ownership, so a second call doesn't copy stale NULL pointers. */ +static wc_test_ret_t fill_signer_twice_test(void) +{ + DecodedCert cert; + Signer* signer1 = NULL; + Signer* signer2 = NULL; + DerBuffer* der = NULL; + byte* tmp = NULL; + size_t bytes; + XFILE file; + wc_test_ret_t ret; + + WOLFSSL_ENTER("fill_signer_twice_test"); + + tmp = (byte*)XMALLOC(FOURK_BUF, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + if (tmp == NULL) + return WC_TEST_RET_ENC_ERRNO; + + /* Load a DER certificate. */ + file = XFOPEN(certExtNc, "rb"); + if (!file) { + ERROR_OUT(WC_TEST_RET_ENC_ERRNO, done); + } + bytes = XFREAD(tmp, 1, FOURK_BUF, file); + XFCLOSE(file); + if (bytes == 0) + ERROR_OUT(WC_TEST_RET_ENC_ERRNO, done); + + /* Create a DerBuffer for FillSigner (needed when WOLFSSL_SIGNER_DER_CERT + * is defined). */ + ret = AllocDer(&der, (word32)bytes, CERT_TYPE, HEAP_HINT); + if (ret != 0) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done); + XMEMCPY(der->buffer, tmp, bytes); + + InitDecodedCert(&cert, tmp, (word32)bytes, 0); + ret = ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL); + if (ret != 0) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done); + + /* After parsing, pubKeyStored should be set and publicKey non-NULL. */ + if (!cert.pubKeyStored || cert.publicKey == NULL) { + ERROR_OUT(WC_TEST_RET_ENC_NC, done); + } + + /* First FillSigner: transfers publicKey and subjectCN ownership. */ + signer1 = MakeSigner(HEAP_HINT); + if (signer1 == NULL) + ERROR_OUT(WC_TEST_RET_ENC_ERRNO, done); + + ret = FillSigner(signer1, &cert, CA_TYPE, der); + if (ret != 0) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done); + + /* signer1 should have received the publicKey. */ + if (signer1->publicKey == NULL) { + ERROR_OUT(WC_TEST_RET_ENC_NC, done); + } + + /* After FillSigner, cert->publicKey should be NULL. */ + if (cert.publicKey != NULL) { + ERROR_OUT(WC_TEST_RET_ENC_NC, done); + } + + /* BUG CHECK: pubKeyStored should have been cleared to 0. + * If it is still set, a second FillSigner would copy a NULL pointer. */ + if (cert.pubKeyStored != 0) { + ERROR_OUT(WC_TEST_RET_ENC_NC, done); + } + + /* Also check subjectCNStored is cleared. */ + if (cert.subjectCNStored != 0) { + ERROR_OUT(WC_TEST_RET_ENC_NC, done); + } + + /* Second FillSigner on the same cert should not copy NULL pointers. */ + signer2 = MakeSigner(HEAP_HINT); + if (signer2 == NULL) + ERROR_OUT(WC_TEST_RET_ENC_ERRNO, done); + + ret = FillSigner(signer2, &cert, CA_TYPE, der); + if (ret != 0) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done); + + /* signer2 should NOT have a publicKey (since cert no longer owns one). */ + if (signer2->publicKey != NULL) { + ERROR_OUT(WC_TEST_RET_ENC_NC, done); + } + +done: + FreeDecodedCert(&cert); + if (signer1 != NULL) + FreeSigner(signer1, HEAP_HINT); + if (signer2 != NULL) + FreeSigner(signer2, HEAP_HINT); + FreeDer(&der); + XFREE(tmp, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + + return ret; +} +#endif /* !NO_ASN_TIME && !NO_RSA && WOLFSSL_TEST_CERT && !NO_FILESYSTEM */ + #if defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_TEST_CERT) && \ !defined(NO_FILESYSTEM) && !defined(NO_RSA) && defined(WOLFSSL_GEN_CERT) WOLFSSL_TEST_SUBROUTINE wc_test_ret_t certext_test(void) diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index ce65340ddb..3ad0b4670b 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2254,6 +2254,11 @@ typedef enum MimeStatus */ #define GetCAByAKID wolfSSL_GetCAByAKID #endif + #define FillSigner wc_FillSigner + #define MakeSigner wc_MakeSigner + #define FreeSigner wc_FreeSigner + #define AllocDer wc_AllocDer + #define FreeDer wc_FreeDer #endif /* WOLFSSL_API_PREFIX_MAP */ WOLFSSL_LOCAL int HashIdAlg(word32 oidSum); @@ -2363,9 +2368,9 @@ WOLFSSL_LOCAL int wc_GetPubX509(DecodedCert* cert, int verify, int* badDate); WOLFSSL_LOCAL const byte* OidFromId(word32 id, word32 type, word32* oidSz); WOLFSSL_LOCAL Signer* findSignerByKeyHash(Signer *list, byte *hash); WOLFSSL_LOCAL Signer* findSignerByName(Signer *list, byte *hash); -WOLFSSL_LOCAL int FillSigner(Signer* signer, DecodedCert* cert, int type, DerBuffer *der); -WOLFSSL_LOCAL Signer* MakeSigner(void* heap); -WOLFSSL_LOCAL void FreeSigner(Signer* signer, void* heap); +WOLFSSL_TEST_VIS int FillSigner(Signer* signer, DecodedCert* cert, int type, DerBuffer *der); +WOLFSSL_TEST_VIS Signer* MakeSigner(void* heap); +WOLFSSL_TEST_VIS void FreeSigner(Signer* signer, void* heap); WOLFSSL_LOCAL void FreeSignerTable(Signer** table, int rows, void* heap); WOLFSSL_LOCAL void FreeSignerTableType(Signer** table, int rows, byte type, void* heap); @@ -2608,11 +2613,11 @@ WOLFSSL_LOCAL int wc_EncryptedInfoParse(EncryptedInfo* info, WOLFSSL_LOCAL int PemToDer(const unsigned char* buff, long longSz, int type, DerBuffer** pDer, void* heap, EncryptedInfo* info, int* keyFormat); -WOLFSSL_LOCAL int AllocDer(DerBuffer** der, word32 length, int type, +WOLFSSL_TEST_VIS int AllocDer(DerBuffer** der, word32 length, int type, void* heap); WOLFSSL_LOCAL int AllocCopyDer(DerBuffer** der, const unsigned char* buff, word32 length, int type, void* heap); -WOLFSSL_LOCAL void FreeDer(DerBuffer** der); +WOLFSSL_TEST_VIS void FreeDer(DerBuffer** der); #ifdef WOLFSSL_ASN_PARSE_KEYUSAGE WOLFSSL_LOCAL int ParseKeyUsageStr(const char* value, word16* keyUsage, From 4294eaa7d6342d9cbdd812f12390650b767df0e8 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Fri, 20 Mar 2026 14:34:36 -0500 Subject: [PATCH 2/2] Fix test failure --- wolfssl/wolfcrypt/asn.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 3ad0b4670b..d23b98ad54 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2257,8 +2257,6 @@ typedef enum MimeStatus #define FillSigner wc_FillSigner #define MakeSigner wc_MakeSigner #define FreeSigner wc_FreeSigner - #define AllocDer wc_AllocDer - #define FreeDer wc_FreeDer #endif /* WOLFSSL_API_PREFIX_MAP */ WOLFSSL_LOCAL int HashIdAlg(word32 oidSum);