Bug 1517025 - Do not allow percent symbol in URL hostnames r=kershaw
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 11 Feb 2019 21:49:19 +0000
changeset 458579 2b799d86aeff7d34406b5022829018b5f91b2910
parent 458578 6c7ae3e0b592577012243038ec85b0b8b6460a05
child 458580 c4c141df786bf5904bf1c6860bb91cc6b24d055a
push id35538
push userbtara@mozilla.com
push dateTue, 12 Feb 2019 05:25:24 +0000
treeherdermozilla-central@09beaf742eae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskershaw
bugs1517025
milestone67.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 1517025 - Do not allow percent symbol in URL hostnames r=kershaw Differential Revision: https://phabricator.services.mozilla.com/D16694
netwerk/base/nsStandardURL.cpp
netwerk/test/unit/test_bug412457.js
netwerk/test/unit/test_bug464591.js
netwerk/test/unit/test_standardurl.js
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -55,30 +55,26 @@ nsIIDNService *nsStandardURL::gIDN = nul
 // may race when reading this values, but that's OK because in the worst
 // case we will just dispatch a noop runnable to the main thread.
 bool nsStandardURL::gInitialized = false;
 
 const char nsStandardURL::gHostLimitDigits[] = {'/', '\\', '?', '#', 0};
 bool nsStandardURL::gPunycodeHost = true;
 
 // Invalid host 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.
-//
 // Note that the array below will be initialized at compile time,
 // so we do not need to "optimize" TestForInvalidHostCharacters.
 //
 constexpr bool TestForInvalidHostCharacters(char c) {
   // Testing for these:
   // CONTROL_CHARACTERS " #/:?@[\\]*<>|\"";
   return (c > 0 && c < 32) ||  // The control characters are [1, 31]
          c == ' ' || c == '#' || c == '/' || c == ':' || c == '?' || c == '@' ||
          c == '[' || c == '\\' || c == ']' || c == '*' || c == '<' ||
-         c == '>' || c == '|' || c == '"';
+         c == '>' || c == '|' || c == '"' || c == '%';
 }
 constexpr ASCIIMaskArray sInvalidHostChars =
     CreateASCIIMask(TestForInvalidHostCharacters);
 
 //----------------------------------------------------------------------------
 // nsStandardURL::nsSegmentEncoder
 //----------------------------------------------------------------------------
 
--- a/netwerk/test/unit/test_bug412457.js
+++ b/netwerk/test/unit/test_bug412457.js
@@ -24,22 +24,22 @@ function run_test() {
   Assert.equal(newURI.asciiHost, "example.com");
 
   newURI = newURI.mutate().setSpec("http://example.com:80").finalize();
   Assert.equal(newURI.asciiHost, "example.com");
 
   newURI = newURI.mutate().setSpec("http://example.com/foo").finalize();
   Assert.equal(newURI.asciiHost, "example.com");
 
-  // Characters that are invalid in the host, shouldn't be decoded.
-  newURI = newURI.mutate().setSpec("http://example.com%3ffoo").finalize();
-  Assert.equal(newURI.asciiHost, "example.com%3ffoo");
-  newURI = newURI.mutate().setSpec("http://example.com%23foo").finalize();
-  Assert.equal(newURI.asciiHost, "example.com%23foo");
-  newURI = newURI.mutate().setSpec("http://example.com%3bfoo").finalize();
-  Assert.equal(newURI.asciiHost, "example.com%3bfoo");
-  newURI = newURI.mutate().setSpec("http://example.com%3a80").finalize();
-  Assert.equal(newURI.asciiHost, "example.com%3a80");
-  newURI = newURI.mutate().setSpec("http://example.com%2ffoo").finalize();
-  Assert.equal(newURI.asciiHost, "example.com%2ffoo");
-  newURI = newURI.mutate().setSpec("http://example.com%00").finalize();
-  Assert.equal(newURI.asciiHost, "example.com%00");
+  // Characters that are invalid in the host
+  Assert.throws(() => { newURI = newURI.mutate().setSpec("http://example.com%3ffoo").finalize(); },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
+  Assert.throws(() => { newURI = newURI.mutate().setSpec("http://example.com%23foo").finalize(); },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
+  Assert.throws(() => { newURI = newURI.mutate().setSpec("http://example.com%3bfoo").finalize(); },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
+  Assert.throws(() => { newURI = newURI.mutate().setSpec("http://example.com%3a80").finalize(); },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
+  Assert.throws(() => { newURI = newURI.mutate().setSpec("http://example.com%2ffoo").finalize(); },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
+  Assert.throws(() => { newURI = newURI.mutate().setSpec("http://example.com%00").finalize(); },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
 }
--- a/netwerk/test/unit/test_bug464591.js
+++ b/netwerk/test/unit/test_bug464591.js
@@ -1,20 +1,22 @@
  // 1.percent-encoded IDN that contains blacklisted character should be converted
  //   to punycode, not UTF-8 string
  // 2.only hostname-valid percent encoded ASCII characters should be decoded
  // 3.IDN convertion must not bypassed by %00
 let reference = [
    ["www.example.com%e2%88%95www.mozill%d0%b0.com%e2%81%84www.mozilla.org",
     "www.example.xn--comwww-re3c.xn--mozill-8nf.xn--comwww-rq0c.mozilla.org"],
-   ["www.mozill%61%2f.org", "www.mozilla%2f.org"], // a slash is not valid in the hostname
-   ["www.e%00xample.com%e2%88%95www.mozill%d0%b0.com%e2%81%84www.mozill%61.org",
-    "www.e%00xample.xn--comwww-re3c.xn--mozill-8nf.xn--comwww-rq0c.mozilla.org"],
 ];
 
+let badURIs = [
+   ["www.mozill%61%2f.org"], // a slash is not valid in the hostname
+   ["www.e%00xample.com%e2%88%95www.mozill%d0%b0.com%e2%81%84www.mozill%61.org"],
+]
+
 let prefData =
   [
     {
       name: "network.enableIDN",
       newVal: true
     },
     {
       name: "network.IDN_show_punycode",
@@ -70,9 +72,14 @@ function run_test() {
   for (let i = 0; i < reference.length; ++i) {
     try {
       let result = stringToURL("http://" + reference[i][0]).host;
       equal(result, reference[i][1]);
     } catch (e) {
       ok(false, "Error testing "+reference[i][0]);
     }
   }
+
+  for (let i = 0; i < badURIs.length; ++i) {
+    Assert.throws(() => { let result = stringToURL("http://" + badURIs[i][0]).host; },
+                /NS_ERROR_MALFORMED_URI/, "bad escaped character");
+  }
 }
--- a/netwerk/test/unit/test_standardurl.js
+++ b/netwerk/test/unit/test_standardurl.js
@@ -1,10 +1,11 @@
 "use strict";
 
+ChromeUtils.import('resource://gre/modules/Services.jsm');
 const gPrefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
 
 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) {
@@ -307,19 +308,19 @@ add_test(function test_accentEncoding()
   run_next_test();
 });
 
 add_test(function test_percentDecoding()
 {
   var url = stringToURL("http://%70%61%73%74%65%62%69%6E.com");
   Assert.equal(url.spec, "http://pastebin.com/");
 
-  // We shouldn't unescape characters that are not allowed in the hostname.
-  url = stringToURL("http://example.com%0a%23.google.com/");
-  Assert.equal(url.spec, "http://example.com%0a%23.google.com/");
+  // Disallowed hostname characters are rejected even when percent encoded
+  Assert.throws(() => { url = stringToURL("http://example.com%0a%23.google.com/"); },
+                /NS_ERROR_MALFORMED_URI/, "invalid characters are not allowed");
   run_next_test();
 });
 
 add_test(function test_hugeStringThrows()
 {
   let prefs = Cc["@mozilla.org/preferences-service;1"]
                 .getService(Ci.nsIPrefService);
   let maxLen = prefs.getIntPref("network.standard-url.max-length");
@@ -691,8 +692,22 @@ add_test(function test_idna_host() {
   equal(url.asciiSpec, "http://user:password@xn--lt-uia.example.org:8080/path?query");
 
   url = stringToURL("http://user:password@www.ält.com:8080/path?query#etc");
   url = url.mutate().setRef("").finalize();
   equal(url.spec, "http://user:password@www.xn--lt-uia.com:8080/path?query");
 
   run_next_test();
 });
+
+add_test(function test_bug1517025() {
+  Assert.throws(() => { let other = stringToURL("https://b%9a/"); },
+                /NS_ERROR_UNEXPECTED/, "bad URI");
+
+  Assert.throws(() => { let other = stringToURL("https://b%9ª/"); },
+                /NS_ERROR_MALFORMED_URI/, "bad URI");
+
+  let base = stringToURL("https://bug1517025.bmoattachments.org/attachment.cgi?id=9033787");
+  Assert.throws(() => { let uri = Services.io.newURI("/\\b%9ª", "windows-1252", base); },
+                /NS_ERROR_MALFORMED_URI/, "bad URI");
+
+  run_next_test();
+});