Skip to content

Compat layer: fix memory leak in X509_get0_pubkey #11467

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

Closed
padelsbach wants to merge 1 commit into from
Closed

Compat layer: fix memory leak in X509_get0_pubkey #11467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5491,6 +5491,12 @@ static void FreeX509Contents(WOLFSSL_X509* x509)

FreeX509Name(&x509->issuer);
FreeX509Name(&x509->subject);
#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)
if (x509->pubKeyCache != NULL) {
wolfSSL_EVP_PKEY_free(x509->pubKeyCache);
x509->pubKeyCache = NULL;
}
#endif
if (x509->pubKey.buffer) {
XFREE(x509->pubKey.buffer, x509->heap, DYNAMIC_TYPE_PUBLIC_KEY);
x509->pubKey.buffer = NULL;
Expand Down
25 changes: 25 additions & 0 deletions src/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6554,6 +6554,31 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509)
}
return key;
}


/* Get the certificate's public key without transferring ownership.
*
* The get0 form returns a pointer into the certificate which the caller must
* not free, It is released with the rest of the certificate's contents.
*
* Note: the first call on a given certificate should not race another; the
* cache is built without a lock, as elsewhere in this layer.
*
* @param [in] x509 Certificate.
* @return Public key on success, NULL on error.
*/
WOLFSSL_EVP_PKEY* wolfSSL_X509_get0_pubkey(WOLFSSL_X509* x509)
{
WOLFSSL_ENTER("wolfSSL_X509_get0_pubkey");

if (x509 == NULL)
return NULL;

if (x509->pubKeyCache == NULL)

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Oct 1, 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.

pubKeyCache is not invalidated when the certificate public key changes · Logic errors

wolfSSL_X509_set_pubkey (src/x509.c:16911-16913) replaces pubKey.buffer and pubKeyOID but leaves pubKeyCache alone. After that call, X509_get0_pubkey keeps returning the old key, so code that verifies or compares with the returned key uses the wrong public key.

Suggested fix: In wolfSSL_X509_set_pubkey, free pubKeyCache and set it to NULL after a successful update, and add a test that calls get0, then set_pubkey, then get0 again.

@philljj philljj Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please review

x509->pubKeyCache = wolfSSL_X509_get_pubkey(x509);

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Oct 1, 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.

Unsynchronized lazy init of pubKeyCache races and leaks on shared X509 · Race conditions

The getter writes pubKeyCache with a check-then-set and no lock. If two threads call X509_get0_pubkey on the same reference-counted X509 (for example a CA cert in a shared store), both allocate a key and one overwrites the other's pointer. The overwritten EVP_PKEY (over 11 KB) is leaked, and the unsynchronized write is a data race.

Suggested fix: Fill the cache under the X509's existing ref/lock mutex, and free the losing allocation, or decode the key once when the certificate is created.
Basis: C11 5.1.2.4p25: two conflicting unsynchronized accesses to the same object, at least one a write, are a data race and undefined behavior.


return x509->pubKeyCache;
}
#endif /* OPENSSL_EXTRA_X509_SMALL */

/* End of smaller subset of X509 compatibility functions. Avoid increasing the
Expand Down
34 changes: 34 additions & 0 deletions tests/api/test_ossl_x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2157,3 +2157,37 @@ int test_wolfSSL_X509_cmp(void)
#endif
return EXPECT_RESULT();
}

/* X509_get0_pubkey returns a key the certificate owns, so the caller does
* not free it and repeated calls return the same pointer. Returning a
* freshly allocated key each time leaks one per call, because nothing is
* left to free it: the caller must not, and the certificate never knew
* about it. */
int test_wolfSSL_X509_get0_pubkey(void)
{
EXPECT_DECLS;
#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && !defined(NO_RSA) && \
!defined(NO_CERTS)
X509* x509 = NULL;
EVP_PKEY* first = NULL;
EVP_PKEY* second = NULL;
int i;

ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(svrCertFile,
WOLFSSL_FILETYPE_PEM));

ExpectNotNull(first = X509_get0_pubkey(x509));
ExpectNotNull(second = X509_get0_pubkey(x509));
/* the certificate owns it, so the same object comes back each time */
ExpectPtrEq(first, second);

/* repeated use must not accumulate allocations */
for (i = 0; i < 100; i++) {
ExpectPtrEq(X509_get0_pubkey(x509), first);
}

/* freeing the certificate releases the key; the caller frees nothing */
X509_free(x509);
#endif
return EXPECT_RESULT();
}
4 changes: 3 additions & 1 deletion tests/api/test_ossl_x509.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ int test_wolfSSL_X509_max_name_constraints(void);
int test_wolfSSL_X509_check_ca(void);
int test_X509_get_signature_nid(void);
int test_wolfSSL_X509_cmp(void);
int test_wolfSSL_X509_get0_pubkey(void);

#define TEST_OSSL_X509_DECLS \
TEST_DECL_GROUP("ossl_x509", test_x509_get_key_id), \
Expand Down Expand Up @@ -93,6 +94,7 @@ int test_wolfSSL_X509_cmp(void);
TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_max_name_constraints), \
TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_check_ca), \
TEST_DECL_GROUP("ossl_x509", test_X509_get_signature_nid), \
TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_cmp)
TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_cmp), \
TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_get0_pubkey)

#endif /* WOLFCRYPT_TEST_OSSL_X509_H */
5 changes: 5 additions & 0 deletions wolfssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5950,6 +5950,11 @@ struct WOLFSSL_X509 {
byte certPolicySet;
byte certPolicyCrit;
#endif /* WOLFSSL_SEP */
#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)
/* Public key owned by this certificate and returned by the get0 form,
* which the caller must not free. Built on first use. */
WOLFSSL_EVP_PKEY* pubKeyCache;
#endif
#if defined(WOLFSSL_QT) || defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)
WOLFSSL_STACK* ext_sk; /* Store X509_EXTENSIONS from wolfSSL_X509_get_ext */
WOLFSSL_STACK* ext_sk_full; /* Store X509_EXTENSIONS from wolfSSL_X509_get0_extensions */
Expand Down
2 changes: 1 addition & 1 deletion wolfssl/openssl/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ typedef STACK_OF(ACCESS_DESCRIPTION) AUTHORITY_INFO_ACCESS;
#define X509_get_subject_name(x) wolfSSL_X509_get_subject_name((WOLFSSL_X509*)(x))
#define X509_REQ_get_subject_name wolfSSL_X509_get_subject_name
#define X509_get_pubkey wolfSSL_X509_get_pubkey
#define X509_get0_pubkey wolfSSL_X509_get_pubkey
#define X509_get0_pubkey wolfSSL_X509_get0_pubkey
#define X509_REQ_get_pubkey wolfSSL_X509_get_pubkey
#define X509_get_notBefore wolfSSL_X509_get_notBefore
#define X509_get0_notBefore wolfSSL_X509_get_notBefore
Expand Down
1 change: 1 addition & 0 deletions wolfssl/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2507,6 +2507,7 @@ WOLFSSL_API WOLFSSL_ASN1_TIME* wolfSSL_X509_CRL_get_nextUpdate(WOLFSSL_X509_CRL*
WOLFSSL_API int wolfSSL_X509_CRL_set_nextUpdate(WOLFSSL_X509_CRL* crl,
const WOLFSSL_ASN1_TIME* time);
WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509);
WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_get0_pubkey(WOLFSSL_X509* x509);
WOLFSSL_API int wolfSSL_X509_CRL_verify(WOLFSSL_X509_CRL* crl, WOLFSSL_EVP_PKEY* pkey);
WOLFSSL_API void wolfSSL_X509_OBJECT_free_contents(WOLFSSL_X509_OBJECT* obj);
WOLFSSL_API WOLFSSL_PKCS8_PRIV_KEY_INFO* wolfSSL_d2i_PKCS8_PKEY_bio(
Expand Down
Loading