[$csp2 adblockpluscore] Issue 6329 - Add the CSP filter type

lib/filterClasses.js

Index: lib/filterClasses.js
diff --git a/lib/filterClasses.js b/lib/filterClasses.js
index 1498ad8db2da7975ef3dce4916ef843d29d51420..67f91699e9496f7b37cdbc11e3337894f72aa8d6 100644
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -97,7 +97,7 @@ Filter.regexpRegExp = /^(@@)?\/.*\/(?:\$~?[\w-]+(?:=[^,\s]+)?(?:,~?[\w-]+(?:=[^,
  * Regular expression that options on a RegExp filter should match
  * @type {RegExp}
  */
-Filter.optionsRegExp = /\$(~?[\w-]+(?:=[^,\s]+)?(?:,~?[\w-]+(?:=[^,\s]+)?)*)$/;
+Filter.optionsRegExp = /\$(~?[\w-]+(?:=[^,]+)?(?:,~?[\w-]+(?:=[^,]+)?)*)$/;
 
 /**
  * Creates a filter of correct type from its text representation - does the
@@ -175,19 +175,49 @@ Filter.normalize = function(text)
   // Remove line breaks and such
   text = text.replace(/[^\S ]/g, "");
 
-  if (/^\s*!/.test(text))
-  {
-    // Don't remove spaces inside comments
+  // Don't remove spaces inside comments
+  if (/^ *!/.test(text))
     return text.trim();
-  }
-  else if (Filter.elemhideRegExp.test(text))
+
+  // Special treatment for element hiding filters, right side is allowed to
+  // contain spaces
+  if (Filter.elemhideRegExp.test(text))
   {
-    // Special treatment for element hiding filters, right side is allowed to
-    // contain spaces
     let [, domain, separator, selector] = /^(.*?)(#@?#?)(.*)$/.exec(text);
-    return domain.replace(/\s/g, "") + separator + selector.trim();
+    return domain.replace(/ /g, "") + separator + selector.trim();

[Manish Jethani, 2018/03/14 15:27:30]

By the way, I tried to find out if / +/g is any better than / /g.

Here's a test I copied and pasted into the Chrome and Firefox consoles:

(function()
{
  let data = new Array(1000000);
  for (let i = 0; i < data.length; i++)
  {
    data[i] = Math.random().toString(36).slice(2).split("").join(
      new Array(Math.ceil(Math.random() * 10)).fill(" ").join("")
    );
  }

  console.log(data.slice(0, 10));

  let startTime = Date.now();
  for (let i = 0; i < data.length; i++)
    data[i] = data[i].replace(/ /g, "");

  console.log("time:", Date.now() - startTime);

  console.log(data.slice(0, 10));
})();

So this prints "time: 2128" or something in that order (slightly faster on
Firefox) for me. If I replace the regular expression with / +/g, it prints
"time: 812" or something in that order (again slightly faster on Firefox). I
think it cuts the execution time in half at least.

I repeated the test several times of course to account for the VM optimizing
that function and variance in general.

So we should go ahead and use / +/g in my opinion.

Note that we can also do the same for /[^\S ]/g above since that too is just a
single character match.

[kzar, 2018/03/14 17:28:59]

Done.

+  }
+
+  // For most regexp filters we strip all whitespace.
+  let strippedText = text.replace(/ /g, "");
+  if (!/csp=/i.test(strippedText))

[Manish Jethani, 2018/03/14 18:10:50]

So how about adding another check here:

  if (strippedText.indexOf("$") == -1 || !/csp=/i.test(strippedText))
    return strippedText;

Because EasyList starts off with a ton of filters with no "$"

Even better:

  let dollarIndex = strippedText.lastIndexOf("$");
  if (dollarIndex == -1 || !/csp=/i.test(strippedText.substring(dollarIndex +
1)))
    return strippedText;

We could probably reuse dollarIndex below (see my other comments).

Also I would change /csp=/i to /\bcsp=/i

[kzar, 2018/03/14 19:48:24]

Seems you're right, of 65k filters EasyList has 47k with no options part. Done.

+    return strippedText;
+
+  let optionsMatch = Filter.optionsRegExp.exec(strippedText);
+  if (!optionsMatch)
+    return strippedText;
+
+  // But since the values of $csp filter options are allowed to contain single
+  // (non trailing) spaces we have to be more careful if they might be present.
+  let beforeOptions = strippedText.substring(0, optionsMatch.index);
+  let optionsText = text;
+  for (let i = beforeOptions.split("$").length; i > 0; i--)

[Manish Jethani, 2018/03/14 18:10:50]

Could you explain this part? I'm not sure what this is supposed to do. If the
idea is to get the part after the last "$" in the original text, we could just
use String.lastIndexOf here.

  let optionsText = text.substring(text.lastIndexOf("$") + 1);

For what it's worth neither the original code nor my suggestion works with the
following case:

  /foo$/$ csp = script-src  http://example.com/?$1=1&$2=2&$3=3

"$" is a valid character in the query string part of the URL:

https://stackoverflow.com/questions/1547899/which-characters-make-a-url-invalid

Thankfully there's no case for having a query string or indeed even the path
part of the URL in the $csp filter. Not yet anyway, but things may change in the
future with the CSP standard. Further the filter author can always encode the
"$" (recommended anyway) so it should be OK.

[kzar, 2018/03/14 19:48:24]

This code uses the index of the options part of the stripped filter text to
figure out what the index of the options part is of the original filter text. It
does that by counting the number of '$'s before the options start in the
stripped string and skipping past that many in the non stripped string.
I've added a test case for that and it worked OK, have I missed something?

+    optionsText = optionsText.substr(optionsText.indexOf("$") + 1);
+
+  let options = [];

[Manish Jethani, 2018/03/14 18:10:50]

Why create a separate array for the stripped version? We could just do this:

  let options = optionsText.split(",");
  for (let i = 0; i < options.length; i++)
    options[i] = options[i].replace(...);

[kzar, 2018/03/14 19:48:24]

Done.

+  for (let option of optionsText.split(","))
+  {
+    let cspMatch = /^( *c *s *p *=)([^,]+)/i.exec(option);

[Manish Jethani, 2018/03/14 18:10:50]

The regular expression here could be /^( *c *s *p *=)(.+)/i because we already
know that there are no commas there.

And why do we need the capturing groups?

  let cspMatch = /^ *c *s *p *=/.exec(option);
  if (cspMatch)
  {
    options.push(
      cspMatch[0].replace(/ /g, "") +
      option.substring(cspMatch[0].length).trim().replace(/ +/g, " ")
    );
  }

I have a feeling that this might actually be faster, though I haven't tested
this.

[kzar, 2018/03/14 19:48:24]

Done.

+    if (cspMatch)
+    {
+      options.push(
+        cspMatch[1].replace(/ /g, "") + cspMatch[2].trim().replace(/ +/g, " ")
+      );
+    }
+    else
+      options.push(option.replace(/ /g, ""));
   }
-  return text.replace(/\s/g, "");
+
+  return beforeOptions + "$" + options.join();
 };
 
 /**
@@ -727,11 +757,12 @@ RegExpFilter.fromText = function(text)
   let sitekeys = null;
   let thirdParty = null;
   let collapse = null;
+  let csp = null;
   let options;
   let match = (text.indexOf("$") >= 0 ? Filter.optionsRegExp.exec(text) : null);
   if (match)
   {
-    options = match[1].toUpperCase().split(",");
+    options = match[1].split(",");
     text = match.input.substr(0, match.index);
     for (let option of options)
     {
@@ -742,12 +773,20 @@ RegExpFilter.fromText = function(text)
         value = option.substr(separatorIndex + 1);
         option = option.substr(0, separatorIndex);
       }
-      option = option.replace(/-/, "_");
+      option = option.replace(/-/, "_").toUpperCase();
       if (option in RegExpFilter.typeMap)
       {
         if (contentType == null)
           contentType = 0;
         contentType |= RegExpFilter.typeMap[option];
+
+        if (option == "CSP" && typeof value != "undefined")
+        {
+          if (csp)
+            csp.push(value);
+          else
+            csp = [value];
+        }
       }
       else if (option[0] == "~" && option.substr(1) in RegExpFilter.typeMap)
       {
@@ -760,7 +799,7 @@ RegExpFilter.fromText = function(text)
       else if (option == "~MATCH_CASE")
         matchCase = false;
       else if (option == "DOMAIN" && typeof value != "undefined")
-        domains = value;
+        domains = value.toUpperCase();
       else if (option == "THIRD_PARTY")
         thirdParty = true;
       else if (option == "~THIRD_PARTY")
@@ -770,7 +809,7 @@ RegExpFilter.fromText = function(text)
       else if (option == "~COLLAPSE")
         collapse = false;
       else if (option == "SITEKEY" && typeof value != "undefined")
-        sitekeys = value;
+        sitekeys = value.toUpperCase();
       else
         return new InvalidFilter(origText, "filter_unknown_option");
     }
@@ -780,8 +819,19 @@ RegExpFilter.fromText = function(text)
   {
     if (blocking)
     {
+      if (csp)
+      {
+        csp = csp.join("; ").toLowerCase();
+
+        // Prevent filters from injecting report-uri or report-to directives
+        // since they are a privacy concern. Regexp based upon reBadCSP[1].
+        // [1] - https://github.com/gorhill/uBlock/blob/67e06f53b4d73df6179f6d320553a55da4ead40e/src/js/static-net-filtering.js#L1362
+        if (/(;|^)\s*report-(to|uri)\b/.test(csp))
+          return new InvalidFilter(origText, "filter_invalid_csp");
+      }
+
       return new BlockingFilter(origText, text, contentType, matchCase, domains,
-                                thirdParty, sitekeys, collapse);
+                                thirdParty, sitekeys, collapse, csp);
     }
     return new WhitelistFilter(origText, text, contentType, matchCase, domains,
                                thirdParty, sitekeys);
@@ -805,6 +855,7 @@ RegExpFilter.typeMap = {
   DOCUMENT: 64,
   WEBSOCKET: 128,
   WEBRTC: 256,
+  CSP: 512,
   XBL: 1,
   PING: 1024,
   XMLHTTPREQUEST: 2048,
@@ -821,9 +872,10 @@ RegExpFilter.typeMap = {
   GENERICHIDE: 0x80000000
 };
 
-// DOCUMENT, ELEMHIDE, POPUP, GENERICHIDE and GENERICBLOCK options shouldn't
-// be there by default
-RegExpFilter.prototype.contentType &= ~(RegExpFilter.typeMap.DOCUMENT |
+// CSP, DOCUMENT, ELEMHIDE, POPUP, GENERICHIDE and GENERICBLOCK options
+// shouldn't be there by default
+RegExpFilter.prototype.contentType &= ~(RegExpFilter.typeMap.CSP |
+                                        RegExpFilter.typeMap.DOCUMENT |
                                         RegExpFilter.typeMap.ELEMHIDE |
                                         RegExpFilter.typeMap.POPUP |
                                         RegExpFilter.typeMap.GENERICHIDE |
@@ -840,16 +892,19 @@ RegExpFilter.prototype.contentType &= ~(RegExpFilter.typeMap.DOCUMENT |
  * @param {string} sitekeys see RegExpFilter()
  * @param {boolean} collapse
  *   defines whether the filter should collapse blocked content, can be null
+ * @param {string} [csp]
+ *   Content Security Policy to inject when the filter matches
  * @constructor
  * @augments RegExpFilter
  */
 function BlockingFilter(text, regexpSource, contentType, matchCase, domains,
-                        thirdParty, sitekeys, collapse)
+                        thirdParty, sitekeys, collapse, csp)
 {
   RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains,
                     thirdParty, sitekeys);
 
   this.collapse = collapse;
+  this.csp = csp;
 }
 exports.BlockingFilter = BlockingFilter;
 
@@ -861,7 +916,13 @@ BlockingFilter.prototype = extend(RegExpFilter, {
    * Can be null (use the global preference).
    * @type {boolean}
    */
-  collapse: null
+  collapse: null,
+
+  /**
+   * Content Security Policy to inject for matching requests.
+   * @type {string}
+   */
+  csp: null
 });
 
 /**