From 3e8ff0a8d4660b50e9c785f101f2bab95b5d6ea7 Mon Sep 17 00:00:00 2001
From: Nicholas Hurley <hurley@mozilla.com>
Date: Fri, 7 Apr 2017 13:18:20 -0700
Subject: [PATCH] Bug 1352146 - Don't allow status phrases in http/2. r=mcmanus

MozReview-Commit-ID: Cf30tUivhnB

--HG--
extra : rebase_source : d3b64d6ee9077f448ff9b8aaaff3d449655f0b62
---
 netwerk/protocol/http/Http2Stream.cpp   | 13 +++++++++++++
 netwerk/test/unit/test_http2.js         |  8 ++++++++
 testing/xpcshell/moz-http2/moz-http2.js |  8 ++++++++
 3 files changed, 29 insertions(+)

diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp
index 09d958b2f9ed..7886e14e02f9 100644
--- a/netwerk/protocol/http/Http2Stream.cpp
+++ b/netwerk/protocol/http/Http2Stream.cpp
@@ -1022,6 +1022,19 @@ Http2Stream::ConvertResponseHeaders(Http2Decompressor *decompressor,
 
   nsresult errcode;
   httpResponseCode = statusString.ToInteger(&errcode);
+
+  // Ensure the :status is just an HTTP status code
+  // https://tools.ietf.org/html/rfc7540#section-8.1.2.4
+  // https://bugzilla.mozilla.org/show_bug.cgi?id=1352146
+  nsAutoCString parsedStatusString;
+  parsedStatusString.AppendInt(httpResponseCode);
+  if (!parsedStatusString.Equals(statusString)) {
+    LOG3(("Http2Stream::ConvertResposeHeaders %p status %s is not just a code",
+          this, statusString.BeginReading()));
+    // Results in stream reset with PROTOCOL_ERROR
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
+
   LOG3(("Http2Stream::ConvertResponseHeaders %p response code %d\n", this, httpResponseCode));
   if (mIsTunnel) {
     LOG3(("Http2Stream %p Tunnel Response code %d", this, httpResponseCode));
diff --git a/netwerk/test/unit/test_http2.js b/netwerk/test/unit/test_http2.js
index a5eba39234e1..ef66530a6737 100644
--- a/netwerk/test/unit/test_http2.js
+++ b/netwerk/test/unit/test_http2.js
@@ -951,6 +951,13 @@ function test_http2_push_userContext3() {
   chan.asyncOpen2(listener);
 }
 
+function test_http2_status_phrase() {
+  var chan = makeChan("https://localhost:" + serverPort + "/statusphrase");
+  var listener = new Http2CheckListener();
+  listener.shouldSucceed = false;
+  chan.asyncOpen2(listener);
+}
+
 function test_complete() {
   resetPrefs();
   do_test_pending();
@@ -996,6 +1003,7 @@ var tests = [ test_http2_post_big
             , test_http2_illegalhpackhard
             , test_http2_folded_header
             , test_http2_empty_data
+            , test_http2_status_phrase
             // Add new tests above here - best to add new tests before h1
             // streams get too involved
             // These next two must always come in this order
diff --git a/testing/xpcshell/moz-http2/moz-http2.js b/testing/xpcshell/moz-http2/moz-http2.js
index 398e59265661..0ba1859d7e13 100644
--- a/testing/xpcshell/moz-http2/moz-http2.js
+++ b/testing/xpcshell/moz-http2/moz-http2.js
@@ -799,6 +799,14 @@ function handleRequest(req, res) {
     res.setHeader("x-client-port", req.remotePort);
   }
 
+  else if (u.pathname === "/statusphrase") {
+    // Fortunately, the node-http2 API is dumb enough to allow this right on
+    // through, so we can easily test rejecting this on gecko's end.
+    res.writeHead("200 OK");
+    res.end(content);
+    return;
+  }
+
   res.setHeader('Content-Type', 'text/html');
   if (req.httpVersionMajor != 2) {
     res.setHeader('Connection', 'close');