Bug 959100 - Detect chunk size overflow in ParseChunkRemaining. r=jduell
☠☠ backed out by 0d8510edd4e6 ☠ ☠
authorDaniel Stenberg <daniel@haxx.se>
Fri, 17 Jan 2014 10:03:44 -0500
changeset 163960 8a5f36d486b48bccc71ecff124317ba16ef78fbb
parent 163959 d95b51c157ec1aeaa9c858514913716d346760a1
child 163961 3687b6b2e48d0a2e9437380ababcfa0b16cf03de
push id38593
push userryanvm@gmail.com
push dateFri, 17 Jan 2014 15:03:48 +0000
treeherdermozilla-inbound@3687b6b2e48d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjduell
bugs959100
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 959100 - Detect chunk size overflow in ParseChunkRemaining. r=jduell
netwerk/protocol/http/nsHttpChunkedDecoder.cpp
netwerk/test/unit/test_chunked_responses.js
netwerk/test/unit/xpcshell.ini
netwerk/test/unit_ipc/test_chunked_responses_wrap.js
--- a/netwerk/protocol/http/nsHttpChunkedDecoder.cpp
+++ b/netwerk/protocol/http/nsHttpChunkedDecoder.cpp
@@ -116,22 +116,31 @@ nsHttpChunkedDecoder::ParseChunkRemainin
             }
             else {
                 mWaitEOF = false;
                 mReachedEOF = true;
                 LOG(("reached end of chunked-body\n"));
             }
         }
         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)) {
-                LOG(("sscanf failed parsing hex on string [%s]\n", buf));
+            // mChunkRemaining is an uint32_t!
+            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;
             }
 
             // we've discovered the last chunk
             if (mChunkRemaining == 0)
                 mWaitEOF = true;
         }
 
new file mode 100644
--- /dev/null
+++ b/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);
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -179,16 +179,17 @@ skip-if = os == "android"
 [test_content_sniffer.js]
 [test_cookie_header.js]
 [test_cookiejars.js]
 [test_cookiejars_safebrowsing.js]
 [test_data_protocol.js]
 [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]
 [test_fallback_no-cache-entry_canceled.js]
 [test_fallback_no-cache-entry_passing.js]
 [test_fallback_redirect-to-different-origin_canceled.js]
 [test_fallback_redirect-to-different-origin_passing.js]
 [test_fallback_request-error_canceled.js]
new file mode 100644
--- /dev/null
+++ b/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");
+}