Bug 1019984 - CSP in CPP: Update Parser to return error when parsing invalid paths (r=sstamm)
authorChristoph Kerschbaumer <mozilla@christophkerschbaumer.com>
Tue, 03 Jun 2014 23:07:27 -0700
changeset 205908 a76d3c8b0f6421112bdb0516108d0651526b3d0e
parent 205907 daae873f90b3252b46426170311921e1e3edfc1c
child 205909 91ad28afdd77610e271e28eac36e572de6904bdc
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstamm
bugs1019984
milestone32.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 1019984 - CSP in CPP: Update Parser to return error when parsing invalid paths (r=sstamm)
content/base/src/nsCSPParser.cpp
content/base/test/TestCSPParser.cpp
--- a/content/base/src/nsCSPParser.cpp
+++ b/content/base/src/nsCSPParser.cpp
@@ -255,16 +255,24 @@ nsCSPParser::subPath(nsCSPHostSrc* aCspH
 
   while (!atEnd() && !peek(DOT)) {
     ++charCounter;
     while (hostChar() || accept(UNDERLINE)) {
       /* consume */
       ++charCounter;
     }
     if (accept(SLASH)) {
+      // do not accept double slashes
+      // see http://tools.ietf.org/html/rfc3986#section-3.3
+      if (accept(SLASH)) {
+        const char16_t* params[] = { mCurToken.get() };
+        logWarningErrorToConsole(nsIScriptError::warningFlag, "couldntParseInvalidSource",
+                                 params, ArrayLength(params));
+        return false;
+      }
       aCspHost->appendPath(mCurValue);
       // Resetting current value since we are appending parts of the path
       // to aCspHost, e.g; "http://www.example.com/path1/path2" then the
       // first part is "/path1", second part "/path2"
       resetCurValue();
     }
     if (charCounter > kSubHostPathCharacterCutoff) {
       return false;
@@ -284,22 +292,28 @@ nsCSPParser::path(nsCSPHostSrc* aCspHost
 
   // Resetting current value and forgetting everything we have parsed so far
   // e.g. parsing "http://www.example.com/path1/path2", then
   // "http://www.example.com" has already been parsed so far
   // forget about it.
   resetCurValue();
 
   if (!accept(SLASH)) {
+    const char16_t* params[] = { mCurToken.get() };
+    logWarningErrorToConsole(nsIScriptError::warningFlag, "couldntParseInvalidSource",
+                             params, ArrayLength(params));
     return false;
   }
   if (atEnd()) {
     return true;
   }
   if (!hostChar()) {
+    const char16_t* params[] = { mCurToken.get() };
+    logWarningErrorToConsole(nsIScriptError::warningFlag, "couldntParseInvalidSource",
+                             params, ArrayLength(params));
     return false;
   }
   return subPath(aCspHost);
 }
 
 bool
 nsCSPParser::subHost()
 {
@@ -459,17 +473,21 @@ nsCSPParser::hostSource()
   if (atEnd()) {
     return cspHost;
   }
 
   // Calling path() to see if there is a path to parse, if an error
   // occurs, path() reports the error; handing cspHost as an argument
   // which simplifies parsing of several paths.
   if (!path(cspHost)) {
-    return cspHost;
+    // If the host [port] is followed by a path, it has to be a valid path,
+    // otherwise we pass the nullptr, indicating an error, up the callstack.
+    // see also http://www.w3.org/TR/CSP11/#source-list
+    delete cspHost;
+    return nullptr;
   }
 
   // Calling fileAndArguments to see if there are any files to parse;
   // if an error occurs, fileAndArguments() reports the error; if
   // fileAndArguments returns true, we have a valid file, so we add it.
   if (fileAndArguments()) {
     cspHost->setFileAndArguments(mCurValue);
   }
--- a/content/base/test/TestCSPParser.cpp
+++ b/content/base/test/TestCSPParser.cpp
@@ -402,38 +402,16 @@ nsresult TestSimplePolicies() {
 nsresult TestPoliciesThatLogWarning() {
 
   static const PolicyTest policies[] =
   {
     { "script-src 'self'; SCRIPT-SRC http://www.example.com",
       "script-src http://www.selfuri.com" },
     { "script-src 'none' test.com; script-src example.com",
       "script-src http://test.com" },
-    { "script-src http://www.example.com//",
-      "script-src http://www.example.com" },
-    { "script-src http://www.example.com/path-1//",
-      "script-src http://www.example.com" },
-    { "script-src http://www.example.com/path-1//path_2",
-      "script-src http://www.example.com" },
-    { "script-src http://www.example.com:88path-1/",
-      "script-src http://www.example.com:88" },
-    { "script-src http://www.example.com:88//",
-      "script-src http://www.example.com:88" },
-    { "script-src http://www.example.com:88//path-1",
-      "script-src http://www.example.com:88" },
-    { "script-src http://www.example.com:88//path-1",
-      "script-src http://www.example.com:88" },
-    { "script-src http://www.example.com:88/.js",
-      "script-src http://www.example.com:88" },
-    { "script-src http://www.example.com:88.js",
-      "script-src http://www.example.com:88" },
-    { "script-src http://www.example.com:*.js",
-      "script-src http://www.example.com:*" },
-    { "script-src http://www.example.com:*.",
-      "script-src http://www.example.com:*" }
   };
 
   uint32_t policyCount = sizeof(policies) / sizeof(PolicyTest);
   return runTestSuite(policies, policyCount, 1);
 }
 
 // ============================= TestBadPolicies ========================
 
@@ -456,17 +434,28 @@ nsresult TestBadPolicies() {
     { "default-src 'unsafe-inlin' ", "" },
     { "default-src :88", "" },
     { "script-src abc::::::88", "" },
     { "asdf http://test.com", ""},
     { "script-src *.*:*", "" },
     { "img-src *::88", "" },
     { "object-src http://localhost:", "" },
     { "script-src test..com", "" },
-    { "script-src sub1.sub2.example+", "" }
+    { "script-src sub1.sub2.example+", "" },
+    { "script-src http://www.example.com//", "" },
+    { "script-src http://www.example.com/path-1//", "" },
+    { "script-src http://www.example.com/path-1//path_2", "" },
+    { "script-src http://www.example.com:88path-1/", "" },
+    { "script-src http://www.example.com:88//", "" },
+    { "script-src http://www.example.com:88//path-1", "" },
+    { "script-src http://www.example.com:88//path-1", "" },
+    { "script-src http://www.example.com:88/.js", "" },
+    { "script-src http://www.example.com:88.js", "" },
+    { "script-src http://www.example.com:*.js", "" },
+    { "script-src http://www.example.com:*.", "" },
   };
 
   uint32_t policyCount = sizeof(policies) / sizeof(PolicyTest);
   return runTestSuite(policies, policyCount, 0);
 }
 
 // ============================= TestGoodGeneratedPolicies ========================