Bug 959100 - Detect chunk size overflow in ParseChunkRemaining. r=jduell

netwerk/protocol/http/nsHttpChunkedDecoder.cpp

@@ -121,12 +121,21 @@ nsHttpChunkedDecoder::ParseChunkRemaining(char *buf,
             }
         }
         else if (*buf) {
+            char *endptr;
+            unsigned long parsedval; // could be 64 bit, could be 32
+
             // ignore any chunk-extensions
             if ((p = PL_strchr(buf, ';')) != nullptr)
                 *p = 0;
 
-            if (!sscanf(buf, "%x", &mChunkRemaining)) {
+            // mChunkRemaining is an uint32_t!
-                LOG(("sscanf failed parsing hex on string [%s]\n", buf));
+            parsedval = strtoul(buf, &endptr, 16);
+            mChunkRemaining = (uint32_t) parsedval;
+
+            if ((endptr == buf) ||
+                (errno == ERANGE) ||
+                (parsedval != mChunkRemaining) ) {
+                LOG(("failed parsing hex on string [%s]\n", buf));
                 return NS_ERROR_UNEXPECTED;
             }
 

netwerk/test/unit/test_chunked_responses.js

@@ -0,0 +1,173 @@
+/*
+ * Test Chunked-Encoded response parsing.
+ */
+
+////////////////////////////////////////////////////////////////////////////////
+// Test infrastructure
+
+Cu.import("resource://testing-common/httpd.js");
+
+XPCOMUtils.defineLazyGetter(this, "URL", function() {
+  return "http://localhost:" + httpserver.identity.primaryPort;
+});
+
+var httpserver = new HttpServer();
+var index = 0;
+var test_flags = new Array();
+var testPathBase = "/chunked_hdrs";
+
+function run_test()
+{
+  httpserver.start(-1);
+
+  do_test_pending();
+  run_test_number(1);
+}
+
+function run_test_number(num)
+{
+  testPath = testPathBase + num;
+  httpserver.registerPathHandler(testPath, eval("handler" + num));
+
+  var channel = setupChannel(testPath);
+  flags = test_flags[num];   // OK if flags undefined for test
+  channel.asyncOpen(new ChannelListener(eval("completeTest" + num),
+                                        channel, flags), null);
+}
+
+function setupChannel(url)
+{
+  var ios = Components.classes["@mozilla.org/network/io-service;1"].
+                       getService(Ci.nsIIOService);
+  var chan = ios.newChannel(URL + url, "", null);
+  var httpChan = chan.QueryInterface(Components.interfaces.nsIHttpChannel);
+  return httpChan;
+}
+
+function endTests()
+{
+  httpserver.stop(do_test_finished);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 1: FAIL because of overflowed chunked size. The parser uses long so
+//         the test case uses >64bit to fail on all platforms.
+test_flags[1] = CL_EXPECT_LATE_FAILURE|CL_ALLOW_UNKNOWN_CL;
+
+function handler1(metadata, response)
+{
+  var body = "12345678123456789\r\ndata never reached";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Transfer-Encoding: chunked\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function completeTest1(request, data, ctx)
+{
+  do_check_eq(request.status, Components.results.NS_ERROR_UNEXPECTED);
+
+  run_test_number(2);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 2: FAIL because of non-hex in chunked length
+
+test_flags[2] = CL_EXPECT_LATE_FAILURE|CL_ALLOW_UNKNOWN_CL;
+
+function handler2(metadata, response)
+{
+  var body = "junkintheway 123\r\ndata never reached";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Transfer-Encoding: chunked\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function completeTest2(request, data, ctx)
+{
+  do_check_eq(request.status, Components.results.NS_ERROR_UNEXPECTED);
+  run_test_number(3);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 3: OK in spite of non-hex digits after size in the length field
+
+test_flags[3] = CL_ALLOW_UNKNOWN_CL;
+
+function handler3(metadata, response)
+{
+  var body = "c junkafter\r\ndata reached";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Transfer-Encoding: chunked\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function completeTest3(request, data, ctx)
+{
+  do_check_eq(request.status, 0);
+  run_test_number(4);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 4: Verify a fully compliant chunked response.
+
+test_flags[4] = CL_ALLOW_UNKNOWN_CL;
+
+function handler4(metadata, response)
+{
+  var body = "c\r\ndata reached\r\n\0\r\n\r\n";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Transfer-Encoding: chunked\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function completeTest4(request, data, ctx)
+{
+  do_check_eq(request.status, 0);
+  run_test_number(5);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 5: A chunk size larger than 32 bit but smaller than 64bit also fails
+// This is probabaly subject to get improved at some point.
+
+test_flags[5] = CL_EXPECT_LATE_FAILURE|CL_ALLOW_UNKNOWN_CL;
+
+function handler5(metadata, response)
+{
+  var body = "123456781\r\ndata never reached";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Transfer-Encoding: chunked\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function completeTest5(request, data, ctx)
+{
+  do_check_eq(request.status, Components.results.NS_ERROR_UNEXPECTED);
+  endTests();
+//  run_test_number(6);
+}

netwerk/test/unit/xpcshell.ini

@@ -184,6 +184,7 @@ skip-if = os == "android"
 [test_dns_service.js]
 [test_dns_localredirect.js]
 [test_duplicate_headers.js]
+[test_chunked_responses.js]
 [test_event_sink.js]
 [test_extract_charset_from_content_type.js]
 [test_force_sniffing.js]

netwerk/test/unit_ipc/test_chunked_responses_wrap.js

@@ -0,0 +1,7 @@
+//
+// Run test script in content process instead of chrome (xpcshell's default)
+//
+
+function run_test() {
+  run_test_in_child("../unit/test_chunked_responses.js");
+}