Skip to content

x509: use an owned public key for -modulus - #300

Open
julek-wolfssl wants to merge 1 commit into
wolfSSL:mainfrom
julek-wolfssl:fix/get0-pubkey-borrowed
Open

julek-wolfssl wants to merge 1 commit into
wolfSSL:mainfrom
julek-wolfssl:fix/get0-pubkey-borrowed

Conversation

@julek-wolfssl

Copy link
Copy Markdown
Member

wolfSSL is moving X509_get0_pubkey() to OpenSSL's borrowed semantics (wolfSSL PR #11428). Under those semantics the key returned is the certificate's own, so the -modulus code freeing it caused a use after free when X509_free() later released the same key.

The -modulus code now calls wolfSSL_X509_get_pubkey(), which returns a reference the caller owns in both old and new wolfSSL. Freeing it remains correct either way.

wolfSSL is moving X509_get0_pubkey() to OpenSSL's borrowed semantics
(wolfSSL PR #11428). The -modulus code freed that key, which with the
new semantics is the certificate's own key, so X509_free() then hit a
use after free. wolfSSL_X509_get_pubkey() returns a reference the
caller owns with both old and new wolfSSL, so freeing it stays correct.

Copilot AI left a comment

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.

Copilot review overview

🟢 Approval recommended

The focused ownership fix matches the updated wolfSSL API contract and existing regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Updates X509 modulus handling for wolfSSL’s borrowed-key semantics, preventing a use-after-free.

Changes:

  • Retrieves an owned public-key reference.
  • Removes the obsolete ownership comment.
File Description
src/​x509/​clu_cert_setup.c Uses an owned key for modulus output and safely frees it.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants