Bug 1352146 - Don't allow status phrases in http/2. r?mcmanus draft
authorNicholas Hurley <hurley@mozilla.com>
Fri, 07 Apr 2017 13:18:20 -0700
changeset 558632 6b2204bda73226ce15bdbcc985fb4a03e91b2da1
parent 558631 2baf4c1f7dfcd427b0de88a1a0cf8a1d780c7013
child 623241 2b111ad804ee4c866120141f4041e9d069d75a70
push id52924
push userbmo:hurley@mozilla.com
push dateFri, 07 Apr 2017 20:33:17 +0000
reviewersmcmanus
bugs1352146
milestone55.0a1
Bug 1352146 - Don't allow status phrases in http/2. r?mcmanus MozReview-Commit-ID: Cf30tUivhnB
netwerk/protocol/http/Http2Stream.cpp
netwerk/test/unit/test_http2.js
testing/xpcshell/moz-http2/moz-http2.js
--- a/netwerk/protocol/http/Http2Stream.cpp
+++ b/netwerk/protocol/http/Http2Stream.cpp
@@ -1017,16 +1017,29 @@ Http2Stream::ConvertResponseHeaders(Http
   decompressor->GetStatus(statusString);
   if (statusString.IsEmpty()) {
     LOG3(("Http2Stream::ConvertResponseHeaders %p Error - no status\n", this));
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
   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));
     if ((httpResponseCode / 100) != 2) {
       MapStreamToPlainText();
     }
   }
 
--- a/netwerk/test/unit/test_http2.js
+++ b/netwerk/test/unit/test_http2.js
@@ -944,16 +944,23 @@ function test_http2_push_userContext2() 
 function test_http2_push_userContext3() {
   var chan = makeChan("https://localhost:" + serverPort + "/push.js");
   chan.loadGroup = loadGroup;
   chan.loadInfo.originAttributes = { userContextId: 1 };
   var listener = new Http2PushListener(true);
   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();
   httpserv.stop(do_test_finished);
   do_test_pending();
   httpserv2.stop(do_test_finished);
 
   do_test_finished();
@@ -989,16 +996,17 @@ var tests = [ test_http2_post_big
             , test_http2_patch
             , test_http2_pushapi_1
             , test_http2_continuations
             , test_http2_blocking_download
             , test_http2_illegalhpacksoft
             , 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
             , test_http2_h11required_stream
             , test_http2_h11required_session
             , test_http2_retry_rst
             , test_http2_wrongsuite
             , test_http2_push_firstparty1
--- a/testing/xpcshell/moz-http2/moz-http2.js
+++ b/testing/xpcshell/moz-http2/moz-http2.js
@@ -793,16 +793,24 @@ function handleRequest(req, res) {
       'content-length' : 1
       });
     pushe.end('1');
   }
   else if (u.pathname.substring(0,8) === "/origin-") { // test_origin.js coalescing
     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');
   }
   res.writeHead(200);
   res.end(content);
 }