Skip to content

Reject CR/LF in OCSP/CRL URLs to block HTTP injection#10628

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

Reject CR/LF in OCSP/CRL URLs to block HTTP injection#10628
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4226

Conversation

@yosuke-wolfssl

@yosuke-wolfssl yosuke-wolfssl commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Description

A TLS peer controls the OCSP AIA URI and CRL Distribution Point URI embedded in its certificate. During certificate verification (ProcessPeerCerts → OCSP/CRL lookup), wolfSSL builds an outbound HTTP request to the responder from that attacker-supplied URL. wolfIO_DecodeUrl accepted CR (\r) and LF (\n) bytes in the host and path, and wolfIO_HttpBuildRequest_ex copied them verbatim into the request line and Host: header. A malicious peer could therefore inject arbitrary HTTP headers or split the request on the connection to the OCSP/CRL responder

Addressed by f_4226.

Changes

  • wolfIO_DecodeUrl: reject CR/LF with WOLFSSL_FATAL_ERROR in all three URL-copy loops — the IPv6 bracket-literal host, the normal host, and the path. Both callers (EmbedOcspLookup, EmbedCrlLookup) already treat a negative return as "unable to decode URL" and abort the lookup.
  • wolfIO_HttpBuildRequest_ex: added a small static wolfIO_UrlHasCrlf helper and reject any path or domainName containing CR/LF before building the request. This is the authoritative check because the CRL path (EmbedCrlLookup → wolfIO_HttpBuildRequestCrl) deliberately sends the raw, uncapped URL as the request target and never passes through the decoder's path loop, so a CRLF byte beyond the 80-byte MAX_URL_ITEM_SIZE cap would otherwise slip past a parser-only fix.
  • Normalized the wolfIO_DecodeUrl error initializer from the literal -1 to WOLFSSL_FATAL_ERROR (same value) so the whole function uses one spelling.
  • New tests are added to exercise the validations.

Testing

  • New test fails against the unpatched source and passes with the fix.
  • OCSP test group (-~test_ocsp): 13/13 pass.
  • Full ./tests/unit.test (--enable-all): all configured tests pass.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 8, 2026
Copilot AI review requested due to automatic review settings June 8, 2026 00:48

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.

Pull request overview

This PR hardens OCSP/CRL URL handling to prevent CR/LF injection (HTTP request splitting / header injection) when building outbound HTTP requests from attacker-controlled certificate URLs during peer verification.

Changes:

  • Reject \r / \n in wolfIO_DecodeUrl() while copying IPv6 bracket hosts, normal hosts, and paths.
  • Add a sink-side CR/LF scan in wolfIO_HttpBuildRequest_ex() to block injection even when URL parsing is bypassed (e.g., CRL absolute-form/raw URL request target).
  • Add a new OCSP API test and register it in the API test runner.

Reviewed changes

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

File Description
src/wolfio.c Adds CR/LF rejection in URL decode and HTTP request building to prevent HTTP injection from certificate-controlled URL components.
tests/api/test_ocsp.c Adds a regression test validating CR/LF rejection in decoded URLs and in the HTTP request builder.
tests/api/test_ocsp.h Declares the new regression test.
tests/api.c Registers the new regression test in the test case list.

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

Comment thread src/wolfio.c
Comment thread tests/api/test_ocsp.c
@yosuke-wolfssl yosuke-wolfssl force-pushed the fix/f_4226 branch 2 times, most recently from b9bf9fa to e08db37 Compare June 8, 2026 03:54

@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 #10628

Scan targets checked: none
Failed targets: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

@parasol-aser

Copy link
Copy Markdown

src/wolfio.cwolfIO_HttpBuildRequest_ex, path copy (the XSTRNCPYXMEMCPY change): pathLen is a signed int that's never checked for negative values, and the maxLen > bufSize guard can't catch it — for pathLen = -1, (word32)pathLen is 0xFFFFFFFF and the unsigned maxLen sum wraps to a small number (e.g. 71) that passes the guard, so XMEMCPY((char*)buf, path, (size_t)pathLen) runs an unbounded copy and overruns buf. The in-tree OCSP/CRL callers always pass non-negative lengths, so this isn't reachable from the cert flow, but it's a real regression for anyone calling the exported wolfIO_HttpBuildRequest directly: the old XSTRNCPY(..., bufSize) was at least bounded by the buffer, whereas the new XMEMCPY corrupts memory immediately.

Fix: reject bad lengths before the length math and copy.

    /* reject invalid lengths before any length math or copy */
    if (path == NULL || pathLen < 0 || bufSize <= 0)
        return 0;

    /* reject CR/LF in the path or host to prevent HTTP header injection or
     * request splitting from attacker-controlled URL components */
    if (wolfIO_UrlHasCrlf(path, pathLen) ||
            wolfIO_UrlHasCrlf(domainName, (int)domainNameLen)) {
        return 0;
    }

A pathLen = -1 case in test_wolfIO_DecodeUrl_crlf_reject asserting the build returns 0 would lock this in.

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.

4 participants