Refactor matching again to properly match all crappy things...

src/const.js

@@ -163,3 +163,14 @@ export const TEMPORARY_APIS = [
   'storage.local',
   'storage.sync',
 ];
+
+// All valid CSP keywords that are options to keys like `default-src` and
+// `script-src`. Used in manifest.json parser for validation.
+// See https://mzl.la/2vwqbGU for more details and allowed options.
+export const CSP_KEYWORD_RE = new RegExp([
+  '(self|none|strict-dynamic|',
+  'unsafe-inline|unsafe-eval|unsafe-hashed-attributes)',
+  // Only match these keywords, anything else is forbidden
+  '(?!.)',
+  '|(sha(256|384|512)-|nonce-)',
+].join(''));

src/parsers/manifestjson.js

@@ -5,7 +5,7 @@ import validate from 'schema/validator';
 import { URL } from 'whatwg-url';
 
 import { getConfig } from 'cli';
-import { MANIFEST_JSON, PACKAGE_EXTENSION } from 'const';
+import { MANIFEST_JSON, PACKAGE_EXTENSION, CSP_KEYWORD_RE } from 'const';
 import log from 'logger';
 import * as messages from 'messages';
 import JSONParser from 'parsers/json';
@@ -184,27 +184,28 @@ export default class ManifestJSONParser extends JSONParser {
         for (let value of values) {
           value = value.trim();
 
-          if (value.endsWith(':') && validProtocols.includes(value)) {
+          if (value.startsWith('moz-extension:')) {
+            // Valid, continue...
+            continue;
+          }
+
+          let hasProtocol = (
+            (value.endsWith(':') && validProtocols.includes(value)) ||
+            (validProtocols.some(x => value.startsWith(x))));
+
+          if (hasProtocol) {
             this.collector.addWarning(messages.MANIFEST_CSP);
             continue;
           }
 
-          try {
-            let url = new URL(value);
+          // strip leading and ending single quotes.
+          value = value.replace(/^[']/, '').replace(/[']$/, '');
 
-            // warn as soon we match a valid URL being whitelisted.
-            // A user doesn't have to prepend a protocol/scheme to a host
-            // so we have to match this a bit wider. This will work since
-            // 'self' and others are required to include the quotes (afair)
-            // which results in an invalid URL.
-
-            if (validProtocols.includes(url.protocol)) {
-              this.collector.addWarning(messages.MANIFEST_CSP);
-            }
-          } catch (e) {
-            if (value.includes('*')) {
-              this.collector.addWarning(messages.MANIFEST_CSP);
-            }
+          if (value === '*' || value.search(CSP_KEYWORD_RE) === -1) {
+            // everything else looks like something we don't understand
+            // / support otherwise is invalid so let's warn about that.
+            this.collector.addWarning(messages.MANIFEST_CSP);
+            continue;
           }
         }
       }

src/schema/formats.js

@@ -84,6 +84,7 @@ function isRelativeUrl(value) {
     // URL is invalid.
     return false;
   }
+
   // If the URL is relative, then the protocol will stay the same, but host
   // could change due to protocol relative. Also check that the URL isn't
   // absolute, since then it is using the dummy protocol we defined.

tests/parsers/test.manifestjson.js

@@ -355,6 +355,9 @@ describe('ManifestJSONParser', function() {
         'default-src ftp:',
         'default-src http://cdn.example.com/my.js',
         'default-src https://cdn.example.com/my.js',
+        'default-src web.example.com',
+        'default-src web.example.com:80',
+        'default-src web.example.com:443',
 
         'script-src *',
         'script-src moz-extension: *',
@@ -365,6 +368,9 @@ describe('ManifestJSONParser', function() {
         'script-src ftp:',
         'script-src http://cdn.example.com/my.js',
         'script-src https://cdn.example.com/my.js',
+        'script-src web.example.com',
+        'script-src web.example.com:80',
+        'script-src web.example.com:443',
 
         // Properly match mixed with other directives
         'script-src https: \'unsafe-eval\'; object-src \'self\'',
@@ -392,14 +398,6 @@ describe('ManifestJSONParser', function() {
         'default-src moz-extension:',
         'script-src moz-extension:',
 
-        // We currently can't properly parse these without producing
-        // loads of false-positives.
-        'default-src web.example.com',
-        'default-src web.example.com:80',
-        'script-src web.example.com',
-        'script-src web.example.com:80',
-        'default-src web.example.com:443',
-
         // Mix with other directives, properly match anyway.
         'script-src \'self\'; object-src \'self\'',
         'script-src \'self\' \'unsafe-eval\'; object-src \'self\'',