1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-23 07:44:48 -05:00

Drop headers with trailing whitespace in header name (#4642)

This relates directly to [an
issue](https://github.com/denoland/deno_std/issues/620) that I initially
raised in `deno_std` awhile back, and was reminded about it today when
the `oak` project popped up on my github recommended repos.

As of now Deno's http servers are vulnerable to the same underlying
issue of go CVE-2019-16276 due to the fact that it's based off of ported
go code from their old standard library. [Here's the commit that fixed
the
CVE.](6e6f4aaf70)

Long story short, some off the shelf proxies and caching servers allow
for passing unaltered malformed headers to backends that they're
fronting. When they pass invalid headers that they don't understand this
can cause issues with HTTP request smuggling. I believe that to this
date, this is the default behavior of AWS ALBs--meaning any server that
strips whitespace from the tail end of header field names and then
interprets the header, when placed behind an ALB, is susceptible to
request smuggling.

The current behavior is actually specifically called out in [RFC
7230](https://tools.ietf.org/html/rfc7230#section-3.2.4) as something
that MUST result in a rejected message, but the change corresponding to
this PR, is more lenient and what both go and nginx currently do, and is
better than the current behavior.
This commit is contained in:
Andrew Stucki 2020-04-06 09:58:46 -04:00 committed by GitHub
parent 703c0b7c17
commit 1e478d73e3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 12 deletions

View file

@ -74,22 +74,16 @@ export class TextProtoReader {
if (kv === Deno.EOF) throw new Deno.errors.UnexpectedEof(); if (kv === Deno.EOF) throw new Deno.errors.UnexpectedEof();
if (kv.byteLength === 0) return m; if (kv.byteLength === 0) return m;
// Key ends at first colon; should not have trailing spaces // Key ends at first colon
// but they appear in the wild, violating specs, so we remove
// them if present.
let i = kv.indexOf(charCode(":")); let i = kv.indexOf(charCode(":"));
if (i < 0) { if (i < 0) {
throw new Deno.errors.InvalidData( throw new Deno.errors.InvalidData(
`malformed MIME header line: ${str(kv)}` `malformed MIME header line: ${str(kv)}`
); );
} }
let endKey = i;
while (endKey > 0 && kv[endKey - 1] == charCode(" ")) {
endKey--;
}
//let key = canonicalMIMEHeaderKey(kv.subarray(0, endKey)); //let key = canonicalMIMEHeaderKey(kv.subarray(0, endKey));
const key = str(kv.subarray(0, endKey)); const key = str(kv.subarray(0, i));
// As per RFC 7230 field-name is a token, // As per RFC 7230 field-name is a token,
// tokens consist of one or more chars. // tokens consist of one or more chars.

View file

@ -94,7 +94,7 @@ test({
}, },
}); });
// Test that we read slightly-bogus MIME headers seen in the wild, // Test that we don't read MIME headers seen in the wild,
// with spaces before colons, and spaces in keys. // with spaces before colons, and spaces in keys.
test({ test({
name: "[textproto] Reader : MIME Header Non compliant", name: "[textproto] Reader : MIME Header Non compliant",
@ -109,9 +109,10 @@ test({
const m = assertNotEOF(await r.readMIMEHeader()); const m = assertNotEOF(await r.readMIMEHeader());
assertEquals(m.get("Foo"), "bar"); assertEquals(m.get("Foo"), "bar");
assertEquals(m.get("Content-Language"), "en"); assertEquals(m.get("Content-Language"), "en");
assertEquals(m.get("SID"), "0"); // Make sure we drop headers with trailing whitespace
assertEquals(m.get("Privilege"), "127"); assertEquals(m.get("SID"), null);
// Not a legal http header assertEquals(m.get("Privilege"), null);
// Not legal http header
assertThrows((): void => { assertThrows((): void => {
assertEquals(m.get("Audio Mode"), "None"); assertEquals(m.get("Audio Mode"), "None");
}); });