fix(h2): keep a request header named __proto__ - #5669
Open
luantaraschi wants to merge 1 commit into
Open
Conversation
buildRequestHeaders probed the accumulator with headers[key], which returns Object.prototype for __proto__ instead of undefined. The header took the already-present branch, its value became '[object Object], pwned', and the assignment then hit the Object.prototype setter, which refuses a string, so nothing reached the wire. The probe uses Object.hasOwn and the writes go through a setHeader helper using Object.defineProperty, the guard parseHeaders already uses.
7 tasks
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
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.
This relates to...
The h2 half of what I flagged in a comment on #5667.
test/prototype-headers.jsalready covers this concern forclient.requestover HTTP/1.Rationale
buildRequestHeadersinlib/dispatcher/client-h2.jsprobes the accumulator by index:For
__proto__that probe returnsObject.prototype, which is truthy, so the header takes the "already present" branch and the value becomes"[object Object], pwned". That string is then assigned through theObject.prototypesetter, which refuses a string, so nothing is stored and the header never reaches the wire.Reproduced over a real HTTP/2 connection, before the change:
__proto__is a valid field name, and the flat array form is exactly what this function receives from the dispatcher.There is no
Object.prototypepollution: the setter refuses the string, so the global prototype is untouched.Changes
The probe uses
Object.hasOwnso an inherited name is not mistaken for an existing header, and all four writes go through asetHeaderhelper usingObject.defineProperty, the guardparseHeadersalready uses inlib/core/util.js.Test added to
test/prototype-headers.js, alongside the HTTP/1 cases: an h2 request carrying__proto__reaches the server with that header, with a control header that already worked.test/+(http2|h2)*.js104 passing with 1 skipped as before, lint clean.Features
N/A
Bug Fixes
An HTTP/2 request no longer drops a header named
__proto__, and no longer corrupts its value with[object Object],first.Breaking Changes and Deprecations
None.
Status