Skip to content

PKCS7: accept any BIO type for detached content in PKCS7_verify #11373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kojo1 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

PKCS7: accept any BIO type for detached content in PKCS7_verify #11373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5d3167
PKCS7: accept any BIO type for detached content in PKCS7_verify
kojo1 Sep 4, 2026
8859779
PKCS7: free contentDynamic when verifying detached content again
kojo1 Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 46 additions & 11 deletions src/ssl_p7p12.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,9 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,
WOLFSSL_X509_STORE* store, WOLFSSL_BIO* in, WOLFSSL_BIO* out, int flags)
{
int i, ret = 0;
int retVal = WOLFSSL_FAILURE;
unsigned char* mem = NULL;
unsigned char* inBuf = NULL;
int memSz = 0;
WOLFSSL_PKCS7* p7 = (WOLFSSL_PKCS7*)pkcs7;
int contTypeLen;
Expand All @@ -798,8 +800,31 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,
return WOLFSSL_FAILURE;

if (in != NULL) {
if ((memSz = wolfSSL_BIO_get_mem_data(in, &mem)) < 0)
return WOLFSSL_FAILURE;
/* Use a memory BIO in place; read any other BIO type into a
* temporary buffer, as wolfSSL_d2i_PKCS7_bio() does. */
if (wolfSSL_BIO_method_type(in) == WOLFSSL_BIO_MEMORY) {
if ((memSz = wolfSSL_BIO_get_mem_data(in, &mem)) < 0)
goto cleanup;
}
else {
if ((memSz = wolfSSL_BIO_get_len(in)) <= 0) {

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detached BIO ingestion rejects empty data and accepts partial reads · API contract violations

wolfSSL_PKCS7_verify() requires a positive length and performs one BIO_read(). Empty signed content is rejected, while a positive short read verifies only a prefix and leaves the remainder unread.

Related known finding #13949 (similar but distinct): Both affect OpenSSL-compatibility API contracts in ssl_p7p12.c, but #13949 is wolfSSL_i2d_PKCS7 output length-query/cursor handling, while this is wolfSSL_PKCS7_verify detached-input ingestion. The faulting operations, root causes, and fixes are separate.

Suggested fix: Accumulate reads until EOF with documented retry handling, permit zero collected bytes, and verify the complete collected content.
Basis: OpenSSL BIO_read(3) permits short positive reads; PKCS7_verify(3) defines indata as detached signed data without a non-empty restriction.

WOLFSSL_MSG("Error getting length of input BIO");
goto cleanup;
}

inBuf = (unsigned char*)XMALLOC((size_t)memSz, p7->pkcs7.heap,
DYNAMIC_TYPE_TMP_BUFFER);
if (inBuf == NULL) {
WOLFSSL_MSG("Error allocating memory for input BIO data");
goto cleanup;
}

if ((memSz = wolfSSL_BIO_read(in, inBuf, memSz)) <= 0) {
WOLFSSL_MSG("Error reading from input BIO");
goto cleanup;
}
Comment on lines +810 to +825
mem = inBuf;
}

p7->pkcs7.content = mem;
p7->pkcs7.contentSz = (word32)memSz;
Comment on lines 829 to 830
Expand All @@ -814,7 +839,7 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,

ret = wc_PKCS7_VerifySignedData(&p7->pkcs7, p7->data, p7->len);
if (ret != 0)
return WOLFSSL_FAILURE;
goto cleanup;

/* Reject a degenerate (certs-only) PKCS#7 with no verified signer. Such an
* object has empty signerInfos, so wc_PKCS7_VerifySignedData() succeeds
Expand All @@ -826,27 +851,27 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,
*/
if (p7->pkcs7.verifyCert == NULL) {
WOLFSSL_MSG("PKCS7 has no verified signer (degenerate/certs-only)");
return WOLFSSL_FAILURE;
goto cleanup;
}

if ((flags & PKCS7_NOVERIFY) != PKCS7_NOVERIFY) {
/* Verify signer certificates */
if (store == NULL || store->cm == NULL) {
WOLFSSL_MSG("No store or store certs, but PKCS7_NOVERIFY not set");
return WOLFSSL_FAILURE;
goto cleanup;
}

ctx = X509_STORE_CTX_new();
if (ctx == NULL) {
WOLFSSL_MSG("Error allocating X509 Store Context");
return WOLFSSL_FAILURE;
goto cleanup;
}

signers = wolfSSL_PKCS7_get0_signers(pkcs7, certs, flags);
if (signers == NULL) {
WOLFSSL_MSG("No signers found to verify");
wolfSSL_X509_STORE_CTX_free(ctx);
return WOLFSSL_FAILURE;
goto cleanup;
}

for (i = 0; i < wolfSSL_sk_X509_num(signers); i++) {
Expand All @@ -856,13 +881,13 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,
WOLFSSL_MSG("Failed to initialize X509 STORE CTX");
wolfSSL_sk_X509_pop_free(signers, NULL);
wolfSSL_X509_STORE_CTX_free(ctx);
return WOLFSSL_FAILURE;
goto cleanup;
}
if (wolfSSL_X509_verify_cert(ctx) != WOLFSSL_SUCCESS) {
WOLFSSL_MSG("Failed to verify signer certificate");
wolfSSL_sk_X509_pop_free(signers, NULL);
wolfSSL_X509_STORE_CTX_free(ctx);
return WOLFSSL_FAILURE;
goto cleanup;
}
}
wolfSSL_sk_X509_pop_free(signers, NULL);
Expand All @@ -875,7 +900,7 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,
if ((p7->pkcs7.contentSz < (word32)contTypeLen) ||
(XMEMCMP(p7->pkcs7.content, contTypeText, contTypeLen) != 0)) {
WOLFSSL_MSG("Error PKCS7 Content-Type not found with PKCS7_TEXT");
return WOLFSSL_FAILURE;
goto cleanup;
}
p7->pkcs7.content += contTypeLen;
p7->pkcs7.contentSz -= contTypeLen;
Expand All @@ -887,7 +912,17 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,

WOLFSSL_LEAVE("wolfSSL_PKCS7_verify", WOLFSSL_SUCCESS);

return WOLFSSL_SUCCESS;
retVal = WOLFSSL_SUCCESS;

cleanup:
if (inBuf != NULL) {
/* content pointed into inBuf */
p7->pkcs7.content = NULL;
p7->pkcs7.contentSz = 0;
XFREE(inBuf, p7->pkcs7.heap, DYNAMIC_TYPE_TMP_BUFFER);
}

return retVal;
}

/**
Expand Down
112 changes: 112 additions & 0 deletions tests/api/test_ossl_p7p12.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,118 @@ int test_wolfSSL_PKCS7_verify_signer_forgery(void)
return EXPECT_RESULT();
}

/* Detached content must be accepted through any BIO type, not just a
* memory BIO. */
int test_wolfSSL_PKCS7_verify_detached_bio(void)
{
EXPECT_DECLS;
#if defined(OPENSSL_ALL) && defined(HAVE_PKCS7) && !defined(NO_BIO) && \
!defined(NO_FILESYSTEM) && !defined(NO_RSA)
const char* signerCertFile = "./certs/server-cert.pem";
const char* signerKeyFile = "./certs/server-key.pem";
const char* caFile = "./certs/ca-cert.pem";
const char* contentFile = "test_pkcs7_detached_content.bin";

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detached-content test races through a shared filename · Race conditions in setup/teardown

The test repeatedly truncates and removes a fixed file in the shared working directory. Concurrent invocations interfere with each other's content and teardown, causing order-dependent failures.

Suggested fix: Use a per-invocation unique temporary path or the project’s temporary-file helper and remove only that fixture during teardown.

byte content[] = "Detached content to verify.";

WOLFSSL_BIO* certBio = NULL;
WOLFSSL_BIO* keyBio = NULL;
WOLFSSL_BIO* caBio = NULL;
WOLFSSL_BIO* signBio = NULL;
WOLFSSL_BIO* inBio = NULL;
X509* signCert = NULL;
EVP_PKEY* signKey = NULL;
X509* caCert = NULL;
X509_STORE* store = NULL;
PKCS7* p7 = NULL;
WOLFSSL_PKCS7* p7Ver = NULL;
byte* der = NULL;
int derSz = 0;
XFILE fp = XBADFILE;

ExpectNotNull(certBio = BIO_new_file(signerCertFile, "r"));
ExpectNotNull(keyBio = BIO_new_file(signerKeyFile, "r"));
ExpectNotNull(caBio = BIO_new_file(caFile, "r"));
ExpectNotNull(signCert = PEM_read_bio_X509(certBio, NULL, 0, NULL));
ExpectNotNull(signKey = PEM_read_bio_PrivateKey(keyBio, NULL, 0, NULL));
ExpectNotNull(caCert = PEM_read_bio_X509(caBio, NULL, 0, NULL));
ExpectNotNull(store = X509_STORE_new());
ExpectIntEQ(X509_STORE_add_cert(store, caCert), 1);

/* create a detached signature over 'content' */
ExpectNotNull(signBio = BIO_new(BIO_s_mem()));
ExpectIntGT(BIO_write(signBio, content, sizeof(content)), 0);
ExpectNotNull(p7 = PKCS7_sign(signCert, signKey, NULL, signBio,
PKCS7_BINARY | PKCS7_DETACHED));
ExpectIntGT((derSz = i2d_PKCS7(p7, &der)), 0);
ExpectNotNull(der);

/* write the detached content out so it can be fed back as a file BIO */
ExpectTrue((fp = XFOPEN(contentFile, "wb")) != XBADFILE);
ExpectIntEQ(XFWRITE(content, 1, sizeof(content), fp), sizeof(content));
if (fp != XBADFILE) {
XFCLOSE(fp);
fp = XBADFILE;
}

/* d2i_PKCS7() cannot be used here: it verifies while decoding and has no
* way to be handed the detached content, so load the bundle directly. */
if (EXPECT_SUCCESS()) {
ExpectNotNull(p7Ver = (WOLFSSL_PKCS7*)PKCS7_new());
if (p7Ver != NULL) {
ExpectNotNull(p7Ver->data = (byte*)XMALLOC((size_t)derSz, NULL,
DYNAMIC_TYPE_PKCS7));
if (p7Ver->data != NULL) {
XMEMCPY(p7Ver->data, der, (size_t)derSz);
p7Ver->len = derSz;
}
}
}

/* a file BIO must work just like a memory BIO */
ExpectNotNull(inBio = BIO_new_file(contentFile, "rb"));
ExpectIntEQ(PKCS7_verify((PKCS7*)p7Ver, NULL, store, inBio, NULL,
PKCS7_BINARY), 1);
BIO_free(inBio);
inBio = NULL;

/* the memory BIO path must keep working */
ExpectNotNull(inBio = BIO_new_mem_buf(content, sizeof(content)));
ExpectIntEQ(PKCS7_verify((PKCS7*)p7Ver, NULL, store, inBio, NULL,
PKCS7_BINARY), 1);
BIO_free(inBio);
inBio = NULL;

/* wrong content must still fail, from a file BIO too */
ExpectTrue((fp = XFOPEN(contentFile, "wb")) != XBADFILE);
ExpectIntEQ(XFWRITE("bogus content", 1, 13, fp), 13);
if (fp != XBADFILE) {
XFCLOSE(fp);
fp = XBADFILE;
}
ExpectNotNull(inBio = BIO_new_file(contentFile, "rb"));
ExpectIntEQ(PKCS7_verify((PKCS7*)p7Ver, NULL, store, inBio, NULL,
PKCS7_BINARY),
WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
BIO_free(inBio);
inBio = NULL;

(void)remove(contentFile);

PKCS7_free((PKCS7*)p7Ver);
PKCS7_free(p7);
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
X509_STORE_free(store);
X509_free(caCert);
X509_free(signCert);
EVP_PKEY_free(signKey);
BIO_free(signBio);
BIO_free(certBio);
BIO_free(keyBio);
BIO_free(caBio);
#endif
return EXPECT_RESULT();
}

/* A degenerate (certs-only) PKCS#7 - one with an empty signerInfos SET and
* therefore no signature at all - must NOT be reported as verified, even when
* the embedded certificate chains to a trusted CA, and even when
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_ossl_p7p12.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ int test_wolfSSL_PKCS7_sign(void);
int test_wolfSSL_PKCS7_verify_signer_forgery(void);
int test_wolfSSL_PKCS7_verify_sid_binding(void);
int test_wolfSSL_PKCS7_verify_degenerate(void);
int test_wolfSSL_PKCS7_verify_detached_bio(void);
int test_wolfSSL_PKCS7_SIGNED_new(void);
int test_wolfSSL_PEM_write_bio_PKCS7(void);
int test_wolfSSL_PEM_write_bio_encryptedKey(void);
Expand All @@ -44,6 +45,7 @@ int test_wolfSSL_PKCS12(void);
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_signer_forgery), \
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_sid_binding), \
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_degenerate), \
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_detached_bio), \
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_SIGNED_new), \
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PEM_write_bio_PKCS7), \
TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PEM_write_bio_encryptedKey), \
Expand Down
3 changes: 3 additions & 0 deletions wolfcrypt/src/pkcs7.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6654,6 +6654,9 @@ static int wc_PKCS7_HandleOctetStrings(wc_PKCS7* pkcs7, byte* in, word32 inSz,
XMEMCPY(pkcs7->stream->content, pkcs7->content, pkcs7->contentSz);
pkcs7->stream->contentSz = pkcs7->contentSz;
}
/* drop the content copy left by a previous verify */
XFREE(pkcs7->contentDynamic, pkcs7->heap, DYNAMIC_TYPE_PKCS7);

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allocation failure leaves detached content pointer dangling · Use-after-free / double-free

contentDynamic can alias content; freeing it leaves content dangling, and replacement-allocation failure makes the next verification read freed memory. Unlike known #9069, this affects the owned content alias, not a caller BIO.

Suggested fix: When freeing an aliased contentDynamic, clear content and contentSz until the replacement allocation succeeds.
Basis: C17 §6.2.4 states that referring to an object outside its lifetime has undefined behavior.

pkcs7->contentDynamic = NULL;
return 0;
}

Expand Down
Loading