Skip to content

[Bug]: Hardening fixes for src/tls13.c #10313

Open
Open
[Bug]: Hardening fixes for src/tls13.c#10313
Assignees
kareem-wolfssl
Labels
bug
@cpsource

Description

@cpsource
cpsource
opened on Apr 25, 2026

Contact Details

page.cal@gmail.com

Version

wolfSSL 5.9.1 (v5.9.1-stable-430-g426dc7bb7 — 430 commits past the v5.9.1 tag, current HEAD 426dc7b)

Description

issue-tmp — Three src/tls13.c hardening fixes

Bundles three independent defensive fixes against the same translation unit.
Each is small (1–10 lines), each has a concrete failure mode, and none of
them are covered by an existing PR or open issue at the time of writing
(see Upstream status below).

  1. Length guards in Tls13_Exporter for outLen and labelLen.
  2. NULL-pointer guards in wolfSSL_get_cipher_name_by_hash.
  3. DoTls13CertificateRequest: defer linking the new CertReqCtx into
    ssl->certReqCtx until after the message is fully validated, so a
    malformed/short CertificateRequest cannot leave a half-validated
    context in the active list.

The bug

1. Tls13_Exporter — unbounded length parameters

src/tls13.c:963-1049

int Tls13_Exporter(WOLFSSL* ssl, unsigned char *out, size_t outLen,
        const char *label, size_t labelLen,
        const unsigned char *context, size_t contextLen)
{
    ...
    /* Sanity check contextLen to prevent truncation when cast to word32. */
    if (contextLen > WOLFSSL_MAX_32BIT) {
        return BAD_FUNC_ARG;
    }
    ...
    ret = Tls13HKDFExpandLabel(ssl, out, (word32)outLen, firstExpand, hashLen,
            protocol, protocolLen, exporterLabel, EXPORTER_LABEL_SZ,
            hashOut, hashLen, (int)hashType);

contextLen is sanity-checked. outLen and labelLen are not. Both are
size_t and both are unconditionally cast to word32 before being passed
into the HKDF layer:

  • Line 1029: (word32)labelLen
  • Line 1044: (word32)outLen

On a 64-bit host, a value > 0xFFFFFFFF silently truncates. The downstream
HKDF wire encoder (wc_Tls13_HKDF_Expand_Label_Alloc in
wolfcrypt/src/kdf.c:538-541) then writes:

data[idx++] = (byte)(okmLen >> 8);   /* 2 bytes */
data[idx++] = (byte)okmLen;
data[idx++] = (byte)(protocolLen + labelLen);  /* 1 byte */

So the API contract is in fact stricter than word32:

  • outLen must fit in two bytes (≤ 0xFFFF); the TLS 1.3 RFC 8446 §7.5
    HkdfLabel encodes length as uint16.
  • protocolLen + labelLen must fit in one byte (≤ 0xFF); the protocol
    label is tls13 (6 bytes), so labelLen must be ≤ 249.

A caller that passes a larger value gets either truncated output (the
HKDF _Alloc variant just truncates and produces wrong key material) or
BUFFER_E (the _ex variant rejects via MAX_TLS13_HKDF_LABEL_SZ,
which is 47 + WC_MAX_DIGEST_SIZE ≈ 111 bytes total). Either way the
caller cannot tell the difference between “HKDF rejected my buffer
sizes” and any other internal error, and on the truncating path the
caller silently gets the wrong key material.

The fix: add structural guards at the API surface, matching the
existing contextLen pattern.

2. wolfSSL_get_cipher_name_by_hash — missing NULL checks

src/tls13.c:14954-14977

const char* wolfSSL_get_cipher_name_by_hash(WOLFSSL* ssl, const char* hash)
{
    const char* name = NULL;
    byte mac = no_mac;
    int i;
    const Suites* suites = WOLFSSL_SUITES(ssl);

    if (XSTRCMP(hash, "SHA256") == 0) {
        ...

This is a WOLFSSL_API function exposed in wolfssl/ssl.h:3230. It is
callable from any consumer of the public ABI. Two NULL paths:

  • WOLFSSL_SUITES(ssl) expands to
    ((ssl)->suites != NULL ? (ssl)->suites : (ssl)->ctx->suites).
    If ssl == NULL, the macro dereferences NULL. If ssl->suites == NULL
    and ssl->ctx == NULL, same story.
  • XSTRCMP(hash, "SHA256") dereferences hash; passing NULL from a
    caller crashes.

Then if mac ends up set, the function dereferences suites->suiteSz and
indexes suites->suites[] without ever checking suites != NULL.

The fix: short-circuit return NULL on any of these inputs.

3. DoTls13CertificateRequest — CertReqCtx linked before validation

src/tls13.c:5936-6041

certReqCtx = (CertReqCtx*)XMALLOC(sizeof(CertReqCtx) +
                  (len == 0 ? 0 : len - 1), ssl->heap,
                  DYNAMIC_TYPE_TMP_BUFFER);
if (certReqCtx == NULL)
    return MEMORY_E;
certReqCtx->next = ssl->certReqCtx;
certReqCtx->len = len;
XMEMCPY(&certReqCtx->ctx, input + *inOutIdx, len);
ssl->certReqCtx = certReqCtx;          /* <-- linked here */
#endif
*inOutIdx += len;

if ((*inOutIdx - begin) + OPAQUE16_LEN > size)
    return BUFFER_ERROR;                /* <-- four error paths follow,
                                           CertReqCtx already in list   */
ato16(input + *inOutIdx, &len);
*inOutIdx += OPAQUE16_LEN;
if ((*inOutIdx - begin) + len > size)
    return BUFFER_ERROR;
if (len == 0)
    return INVALID_PARAMETER;
if ((ret = TLSX_Parse(ssl, input + *inOutIdx, len, certificate_request,
                                                            &peerSuites))) {
    return ret;
}

The newly allocated CertReqCtx is appended to ssl->certReqCtx before
the rest of the message is validated. After that point there are four
distinct error paths (three BUFFER_ERROR, one INVALID_PARAMETER,
plus whatever TLSX_Parse returns). Each one returns up the stack with
a half-validated context already wired into the active list.

Two consequences:

  • Eventual cleanup masks the problem. The chain is freed when
    SSL_ResourceFree runs (src/internal.c:9030-9034), so on a failed
    handshake that immediately tears down the SSL there is no observable
    leak. But under post-handshake auth a server-supplied malformed
    request that fails after linking still leaves an entry in the list
    for the next legitimate SendTls13Certificate call to consume
    (src/tls13.c:9298-9300), so the client may then reply to the
    attacker’s unvalidated context bytes rather than failing cleanly.
  • Defensive correctness. The function’s contract is: on a 0
    return, the message is accepted and the context is registered.
    Any other return code means the message was rejected. The current
    code violates that contract for every non-zero exit reached after
    line 5979.

The fix: defer the ssl->certReqCtx = certReqCtx; linkage until after
all validation and TLSX_Parse succeed. Free the locally-held block on
every error path between allocation and linkage.

Upstream status

Searched wolfSSL/wolfssl PRs and issues (open and closed) on
2026-04-25. No PR or issue is currently open or merged that addresses
any of these three concerns.

Concern Search terms Result
Exporter length guards Tls13_Exporter, TLS 1.3 exporter, outLen labelLen None
wolfSSL_get_cipher_name_by_hash NULL wolfSSL_get_cipher_name_by_hash, get_cipher_name_by_hash null None
DoTls13CertificateRequest CertReqCtx leak DoTls13CertificateRequest, CertReqCtx Adjacent only

Adjacent prior work on the same function:

Root cause

All three follow the same shape: an API function trusts its caller for
input shape that the caller cannot guarantee.

  1. Exporter: the public wolfSSL_export_keying_material (src/ssl.c:5989)
    takes size_t parameters, the underlying HKDF wire encoder takes
    uint8/uint16 lengths, and there is no checked narrowing in the
    middle layer. The contextLen check at tls13.c:1035 was added; the
    matching outLen/labelLen checks were not.
  2. wolfSSL_get_cipher_name_by_hash is a thin lookup function written
    under the assumption that it is only called mid-handshake from
    internal PSK code with a non-NULL ssl. Public ABI exposure breaks
    that assumption.
  3. DoTls13CertificateRequest linkage was placed early so the failure
    path could rely on SSL_ResourceFree for cleanup — but that puts
    unvalidated state into the active list, which is observable while
    the SSL is still alive.

Proposed fix

Three small edits in src/tls13.c. Total: ~30 lines added, ~1 line moved.

1. Tls13_Exporter — add structural guards

/* Sanity check contextLen to prevent truncation when cast to word32. */
if (contextLen > WOLFSSL_MAX_32BIT) {
    return BAD_FUNC_ARG;
}

/* outLen is encoded as uint16 in HkdfLabel (RFC 8446 §7.5). */
if (outLen > WOLFSSL_MAX_16BIT) {
    return BAD_FUNC_ARG;
}
/* protocolLen + labelLen is encoded in a single byte; protocol is
 * "tls13 " (6 bytes), so labelLen must leave room. */
if (labelLen > (size_t)(0xFF - protocolLen)) {
    return BAD_FUNC_ARG;
}

2. wolfSSL_get_cipher_name_by_hash — early NULL return

const char* wolfSSL_get_cipher_name_by_hash(WOLFSSL* ssl, const char* hash)
{
    const char* name = NULL;
    byte mac = no_mac;
    int i;
    const Suites* suites;

    if (ssl == NULL || hash == NULL)
        return NULL;
    suites = WOLFSSL_SUITES(ssl);
    if (suites == NULL)
        return NULL;
    ...

3. DoTls13CertificateRequest — defer linkage

Allocate the context locally, perform every validation step against the
local pointer, and only assign to ssl->certReqCtx immediately before
the success exit. On any error path between allocation and linkage,
XFREE the local pointer.

Caveats and risks

  • The exporter guards reject inputs that would previously have either
    silently truncated or hit BUFFER_E deeper in HKDF. No legitimate
    caller passes ≥ 64 KiB of derived key material out of one exporter
    call — the TLS 1.3 spec does not even encode that — so behavioral
    impact on real callers is nil. Bad callers now get a deterministic
    BAD_FUNC_ARG instead of an HKDF-internal error code.
  • The NULL guard on wolfSSL_get_cipher_name_by_hash cannot regress
    any legitimate caller; a NULL ssl already crashed.
  • The deferred-linkage edit changes the moment ssl->certReqCtx is
    observable to other code on the same SSL object. Callers that read
    ssl->certReqCtx mid-DoTls13CertificateRequest would behave
    differently — but no such caller exists; DoTls13CertificateRequest
    is single-threaded with respect to its own SSL.

Tests to add

  • A unit test that calls Tls13_Exporter with outLen = 0x10000 and
    asserts BAD_FUNC_ARG.
  • A unit test that calls Tls13_Exporter with labelLen = 250 and
    asserts BAD_FUNC_ARG.
  • A unit test that calls wolfSSL_get_cipher_name_by_hash(NULL, "SHA256")
    and asserts the function returns NULL without crashing, and the
    same for a NULL hash.
  • A wire-level test for DoTls13CertificateRequest that injects a
    truncated post-allocation message and asserts (a) the function
    returns an error and (b) ssl->certReqCtx is unchanged from before
    the call. This is harder to fixture in the existing test harness;
    the C reproducer in this directory exercises the same condition by
    calling the function directly.

Files touched by the proposed patch

File Lines Severity
src/tls13.c +27 / −1 Defensive (no known exploit, observable misbehavior on malformed/oversized inputs)

Appendix — end-to-end verification

The test harness in test.sh performs the following passes against an
unpatched and a patched tree:

  1. Build wolfSSL with --enable-harden-tls --enable-postauth --enable-keylog-export --enable-keying-material --enable-psk.
  2. Compile issue-tmp-test.c against the resulting library.
  3. Run the reproducer:
    • Case A: Tls13_Exporter-style overflow (calls
      wolfSSL_export_keying_material with outLen = 0x10000).
      BEFORE: returns WOLFSSL_SUCCESS (silent truncation in HKDF wire
      buffer) or unspecified error. AFTER: returns WOLFSSL_FAILURE
      consistently.
    • Case B: wolfSSL_get_cipher_name_by_hash(NULL, "SHA256").
      BEFORE: SIGSEGV. AFTER: returns NULL.
    • Case C: simulate a malformed DoTls13CertificateRequest
      directly. BEFORE: ssl->certReqCtx non-NULL after error.
      AFTER: ssl->certReqCtx remains NULL after error.

PASS gate: all three cases must show the post-patch behavior on the
patched tree.

Running the test harness

cd wolfssl-issues/issue-tmp
./test.sh

The script resets src/tls13.c to the clean tree before BEFORE,
applies issue-tmp.patch for AFTER, restores the tree on every
exit path, and prints a four-line colored summary.

The rest of the files mentioned are at https://github.com/CpsourceInc/wolfssl-issues.git

Reproduction steps

No response

Relevant log output

Metadata
Metadata
Assignees
Labels
bug
Type
No type
Projects
No projects
Milestone
No milestone
Relationships
None yet
Development
No branches or pull requests
Issue actions