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.](https://github.com/golang/go/commit/6e6f4aaf70c8b1cc81e65a26332aa9409de03ad8)

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.

2 files changed, 7 insertions, 12 deletions

diff --git a/std/textproto/mod.ts b/std/textproto/mod.ts
index 06ffb7364..218ec9d44 100644
--- a/std/textproto/mod.ts
+++ b/std/textproto/mod.ts
@@ -74,22 +74,16 @@ export class TextProtoReader {
       if (kv === Deno.EOF) throw new Deno.errors.UnexpectedEof();
       if (kv.byteLength === 0) return m;
 
-      // Key ends at first colon; should not have trailing spaces
-      // but they appear in the wild, violating specs, so we remove
-      // them if present.
+      // Key ends at first colon
       let i = kv.indexOf(charCode(":"));
       if (i < 0) {
         throw new Deno.errors.InvalidData(
           `malformed MIME header line: ${str(kv)}`
         );
       }
-      let endKey = i;
-      while (endKey > 0 && kv[endKey - 1] == charCode(" ")) {
-        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,
       // tokens consist of one or more chars.
diff --git a/std/textproto/test.ts b/std/textproto/test.ts
index 5833935ba..b9947ab33 100644
--- a/std/textproto/test.ts
+++ b/std/textproto/test.ts
@@ -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.
 test({
   name: "[textproto] Reader : MIME Header Non compliant",
@@ -109,9 +109,10 @@ test({
     const m = assertNotEOF(await r.readMIMEHeader());
     assertEquals(m.get("Foo"), "bar");
     assertEquals(m.get("Content-Language"), "en");
-    assertEquals(m.get("SID"), "0");
-    assertEquals(m.get("Privilege"), "127");
-    // Not a legal http header
+    // Make sure we drop headers with trailing whitespace
+    assertEquals(m.get("SID"), null);
+    assertEquals(m.get("Privilege"), null);
+    // Not legal http header
     assertThrows((): void => {
       assertEquals(m.get("Audio Mode"), "None");
     });