Repository navigation
fix(runtime): reject CR/LF header names/values in the curl repair hop - #14
Merged
Merged
Conversation
curlFetch built -H arguments from raw header names and values, and curl
puts an embedded CRLF on the wire as extra request lines (verified with
the system curl 8.7.1 against a local listener): a page-supplied
tiny.fetch({ headers: { 'X-Evil': '1\r\nX-Injected: yes' } }) on a
root-path URL — or any request the native fetch throws on — reached the
server with forged headers. WHATWG fetch rejects CR/LF/NUL in header
names and values with a TypeError; enforce the same rule here so both
fetch paths agree and a header can't smuggle request lines through the
repair hop.
Verified under tjs: before the fix the injected header lands on the
wire; after, fetch() rejects with 'Invalid header value' before curl is
spawned.
…uded txiki 26.6.0's native fetch also puts a CRLF in a header value on the wire as a separate header, so the curl-hop check alone left the common path open. Validate once at the top of fetchRepaired (and again in curlFetch): names must be RFC 9110 tokens — a ':' in a name made curl send a different header, Host included — and values reject CR/LF/NUL. Also: [[k, v]] header arrays are read as pairs (Array.forEach made them index/pair), and empty values reach curl as 'Name;' instead of 'Name:', which curl reads as remove-this-header. Refs tarwin#11 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks @slabbdev with some small changes this has been merged and will be in the next release |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11 (section 2 — the curl repair hop).
What
curlFetchbuilt its-Harguments from raw header names and values. curl puts an embedded CRLF in an-Hargument on the wire as extra request lines (verified with the system curl 8.7.1 against a local listener), so a page-suppliedtiny.fetch({ headers: { 'X-Evil': '1\r\nX-Injected: yes' } })reached the server with forged headers whenever the request routed through the repair hop (root-path URLs — the bug-A case — or any URL the native fetch throws on, bug B).WHATWG fetch rejects CR/LF/NUL in header names and values with a
TypeError; this makes the curl hop enforce the same rule, so both fetch paths agree and a header can't smuggle request lines through the repair step.Verification
Under tjs (0.42.0 checkout, macOS 26), request to a path-
/URL with the header{'X-Evil': '1\r\nX-Injected: yes'}, server capturing the raw request:fetch()resolved 200 and the wire carriedX-Evil: 1followed byX-Injected: yesas a separate headerfetch()rejects withTypeError: Invalid header valuebefore curl is spawned; nothing reaches the server(Plain headers keep flowing unchanged — the check only fires on CR/LF/NUL.)
Note
One thing I could not verify from this repo: whether txiki's own native fetch validates header values the same way. If it doesn't, the same check may be worth lifting into
doFetchso both paths are covered at the page boundary too — happy to add it here or as a follow-up.