Newlines in HTTP request method results in the injection of HTTP headers in requests

[aegarbutt]

The http request method does not have any validation applied to it before constructing the first line of an HTTP request (https://github.com/joyent/node/blob/master/lib/_http_client.js#L129). A carefully constructed method, such as
GET / HTTP/1.1\r\nX-Foobar: Bazbang\r\nX-Discard:
if passed to http.request(method, '/intendedpath') would result in an HTTP request like:

GET / HTTP/1.1
X-FOOBAR: BAZBANG
X-DISCARD: /intendedpath HTTP/1.1

The forced uppercasing that occurs at https://github.com/joyent/node/blob/master/lib/_http_client.js#L89 can be bypassed by URL encoding the method.

Security impact occurs when a server constructs and submits HTTP requests from client provided data.

HTTP request methods are limited in the HTTP/1.0 and 1.1 RFCs to token, which is defined as:

token          = 1*<any CHAR except CTLs or tspecials>
CTL            = <any US-ASCII control character
                    (octets 0 - 31) and DEL (127)>
tspecials      = "(" | ")" | "<" | ">" | "@"
               | "," | ";" | ":" | "\" | <">
               | "/" | "[" | "]" | "?" | "="
               | "{" | "}" |  SP |  HT

Header injection within headers themselves is accounted for at:
https://github.com/joyent/node/blob/master/lib/_http_outgoing.js#L296-L297

[aegarbutt]

This issue was tracked down with the help of fin1te (https://github.com/fin1te)

[jasnell]

I can confirm this but the syntax appears to be a bit off above... Verified by using nc -l 9999 then doing http.request({host:'localhost', port:9999,method:'GET /foo\r\nX-Boo: ', path:'/bar'}, function(e,d) {}).end();. The nc output shows GET /FOO with X-Boo: /bar.

There would be a slight performance hit to consider, but checking the options.method against var token = /^[^\x00-\x31\x127\(\)<>@,;:\\"/\[\]\?=\{}]+$/ would likely be a good idea.

[misterdjules]

Thank you very much for reporting this issue. The best place to report an issue that could have any security impact on users is to send an email to security@nodejs.org. Hopefully, more information on how to report potential security issues will soon be available on Node.js' website.

[jasnell]

Fortunately, security impact for this one ought to be fairly minimal. But definitely needs to be fixed.

[aegarbutt]

@jasnell Sorry about that. I couldn't find an appropriate reporting page, the security impact seemed relatively minimal except in a very unusual use case we hit, and the issue on HTTP header injection was in the archives.

[brendanashworth]

This has been fixed in master as of nodejs/node@6192c98. @jasnell is this something you'd like to see backported to 0.10 or 0.12?

[jasnell]

Yes. To v0.12 at least.
On Oct 4, 2015 3:35 PM, "Brendan Ashworth" notifications@github.com wrote:

This has been fixed in master as of nodejs/node@6192c98
nodejs/node@6192c98.
@jasnell https://github.com/jasnell is this something you'd like to see
backported to 0.10 or 0.12?

—
Reply to this email directly or view it on GitHub
#8947 (comment)
.