Bug 1337629 - Restrict allowed hostname characters r=mcmanus a=gchang
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 09 Feb 2017 01:55:49 +0100
changeset 376177 3e192f30cabc4b71e2bafb971d6a4ab952388158
parent 376176 8750d50052adc918390f6ff5ce5f5aa793e028ea
child 376178 b47862d310fa5c4e75d88fe5f855b4effde10454
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus, gchang
bugs1337629
milestone53.0a2
Bug 1337629 - Restrict allowed hostname characters r=mcmanus a=gchang MozReview-Commit-ID: H8u2C5oSiT9
netwerk/base/nsStandardURL.cpp
netwerk/test/unit/test_standardurl.js
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -615,17 +615,17 @@ nsStandardURL::ValidIPv6orHostname(const
     }
 
     if (openBracket || closeBracket) {
         // Fail if only one of the brackets is present
         return false;
     }
 
     const char *end = host + length;
-    if (end != net_FindCharInSet(host, end, "\t\n\v\f\r #/:?@[\\]")) {
+    if (end != net_FindCharInSet(host, end, CONTROL_CHARACTERS " #/:?@[\\]*<>|\"")) {
         // We still allow % because it is in the ID of addons.
         // Any percent encoded ASCII characters that are not allowed in the
         // hostname are not percent decoded, and will be parsed just fine.
         return false;
     }
 
     return true;
 }
--- a/netwerk/test/unit/test_standardurl.js
+++ b/netwerk/test/unit/test_standardurl.js
@@ -1,25 +1,27 @@
+"use strict";
+
 const StandardURL = Components.Constructor("@mozilla.org/network/standard-url;1",
                                            "nsIStandardURL",
                                            "init");
 const nsIStandardURL = Components.interfaces.nsIStandardURL;
 
 function symmetricEquality(expect, a, b)
 {
   /* Use if/else instead of |do_check_eq(expect, a.spec == b.spec)| so
      that we get the specs output on the console if the check fails.
    */
   if (expect) {
     /* Check all the sub-pieces too, since that can help with
        debugging cases when equals() returns something unexpected */
     /* We don't check port in the loop, because it can be defaulted in
        some cases. */
     ["spec", "prePath", "scheme", "userPass", "username", "password",
-     "hostPort", "host", "path", "filePath", "param", "query",
+     "hostPort", "host", "path", "filePath", "query",
      "ref", "directory", "fileName", "fileBaseName", "fileExtension"]
       .map(function(prop) {
 	dump("Testing '"+ prop + "'\n");
 	do_check_eq(a[prop], b[prop]);
       });  
   } else {
     do_check_neq(a.spec, b.spec);
   }
@@ -425,8 +427,22 @@ add_test(function test_ipv4Normalize()
   var spec;
   for (spec of nonIPv4s) {
     url = stringToURL(spec);
     do_check_eq(url.spec, spec);
   }
 
   run_next_test();
 });
+
+add_test(function test_invalidHostChars() {
+  var url = stringToURL("http://example.org/");
+  for (let i = 0; i <= 0x20; i++) {
+    Assert.throws(() => { url.host = "a" + String.fromCharCode(i) + "b"; }, "Trying to set hostname containing char code: " + i);
+  }
+  for (let c of "@[]*<>|:\"") {
+    Assert.throws(() => { url.host = "a" + c; }, "Trying to set hostname containing char: " + c);
+  }
+
+  // It also can't contain /, \, #, ?, but we treat these characters as
+  // hostname separators, so there is no way to set them and fail.
+  run_next_test();
+});