Skip to content

Validate X25519 public key length before reading input buffer - #457

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_7489
Open

Validate X25519 public key length before reading input buffer#457
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_7489

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Problem

wp_x25519_import_public() dereferences in[CURVE25519_KEYSIZE - 1] and, when that byte's top bit is set, does a 32-byte XMEMCPY(data, in, CURVE25519_KEYSIZE) — all before inLen is ever examined. The length is only validated later, inside wc_curve25519_import_public_ex().

Neither caller bounds the length first: wp_ecx_import() takes pubData/len straight from OSSL_PKEY_PARAM_PUB_KEY, and wp_ecx_set_params() does the same for OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY. Any X25519 public key shorter than 32 bytes causes a read of up to 31 bytes past the caller's buffer, reachable from EVP_PKEY_fromdata() or EVP_PKEY_set1_encoded_public_key().

Closes f-7489.

Fix (src/wp_ecx_kmgmt.c)

Reject a bad length before in is touched:

    /* Length must be checked before in is read below. */
    if ((in == NULL) || (inLen != CURVE25519_KEYSIZE)) {
        return BAD_FUNC_ARG;
    }

BAD_FUNC_ARG is what the wolfCrypt imports already return for this condition, so callers see no new error class. X25519 is the only affected type — wc_curve448_import_public_ex(), wc_ed25519_import_public_ex() and wc_ed448_import_public_ex() all validate length before reading, and the Ed25519/Ed448 wrappers delegate immediately.

Test harness (test/test_ecx.c)

test_ecx_import_short_pub drives a 4-byte heap buffer (filled 0xff so the top-bit branch is taken) through both entry points: EVP_PKEY_fromdata() with OSSL_PKEY_PARAM_PUB_KEY, and EVP_PKEY_set1_encoded_public_key(). Modelled on the neighbouring test_ecx_import_zero_priv. The heap allocation is what lets ASan see the over-read; the Sanitizers workflow runs make test under ASan+UBSan with -fno-sanitize-recover=all on every PR.

Verification

  • Build clean, no new warnings.
  • Unit suite: 199 passed, 0 failed. All 12 ECX tests pass, confirming valid 32/56/57-byte imports still work.
  • Negative control: with the guard reverted, the new test reports heap-buffer-overflow READ at wp_x25519_import_public via wp_ecx_importEVP_PKEY_fromdata.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Aug 3, 2026
Copilot AI review requested due to automatic review settings August 3, 2026 06:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a reachable out-of-bounds read when importing X25519 public keys by validating the input length before any dereference/copy of the caller-provided buffer, and adds a regression test that would previously trip ASan.

Changes:

  • Add an early guard in wp_x25519_import_public() to reject NULL input or a non-32-byte public key length before reading in[31] or copying 32 bytes.
  • Add a new unit test that exercises both X25519 public-key import entry points with a short (heap) buffer to ensure the over-read cannot occur.
  • Register the new test in the unit test declarations list.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/wp_ecx_kmgmt.c Adds pre-read length validation in X25519 public key import to prevent OOB reads.
test/test_ecx.c Adds ASan-visible regression test for undersized X25519 public key import via both OpenSSL entry points.
test/unit.c Registers the new ECX test in the unit test table.
test/unit.h Declares the new test function prototype.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #457

Scan targets checked: wolfprovider-bugs, wolfprovider-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread test/test_ecx.c
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.

5 participants