Scriptlet argument escaping (#3588)

* Scriptlet argument escaping Scriptlets comming from CDN maybe have escaped arguments for sake of backward compatibility with older adblocker engines - this makes them hard to read and debug, so they should be unescaped before injection. Scriptlets arguments that have escape sequences require the escape charactes to be escaped, otherwise when injected they would be evaluated
ghostery · Nov 10, 2023 · 684dba9 · 684dba9
1 parent 393951f
commit 684dba9
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 20 deletions.
diff --git a/packages/adblocker/src/filters/cosmetic.ts b/packages/adblocker/src/filters/cosmetic.ts
@@ -41,6 +41,9 @@ import { HTMLSelector, extractHTMLSelectorFromRule } from '../html-filtering';
 const EMPTY_TOKENS: [Uint32Array] = [EMPTY_UINT32_ARRAY];
 export const DEFAULT_HIDDING_STYLE: string = 'display: none !important;';
 
+const REGEXP_UNICODE_COMMA = new RegExp(/\\u002C/, 'g');
+const REGEXP_UNICODE_BACKSLASH = new RegExp(/\\u005C/, 'g');
+
 /**
  * Given a `selector` starting with either '#' or '.' check if what follows is
  * a simple CSS selector: /^-?[_a-zA-Z]+[_a-zA-Z0-9-]*$/
@@ -748,16 +751,20 @@ export default class CosmeticFilter implements IFilter {
       return undefined;
     }
 
-    const args = parts.slice(1).map((part) => {
-      if (
-        (part.startsWith(`'`) && part.endsWith(`'`)) ||
-        (part.startsWith(`"`) && part.endsWith(`"`))
-      ) {
-        return part.substring(1, part.length - 1);
-      }
-      return part;
-    });
-
+    const args = parts
+      .slice(1)
+      .map((part) => {
+        if (
+          (part.startsWith(`'`) && part.endsWith(`'`)) ||
+          (part.startsWith(`"`) && part.endsWith(`"`))
+        ) {
+          return part.substring(1, part.length - 1);
+        }
+        return part;
+      })
+      .map((part) =>
+        part.replace(REGEXP_UNICODE_COMMA, ',').replace(REGEXP_UNICODE_BACKSLASH, '\\'),
+      );
     return { name: parts[0], args };
   }
 
@@ -772,7 +779,9 @@ export default class CosmeticFilter implements IFilter {
     let script = js.get(name);
     if (script !== undefined) {
       for (let i = 0; i < args.length; i += 1) {
-        script = script.replace(`{{${i + 1}}}`, args[i]);
+        // escape some characters so they wont get evaluated with escape characters during script injection
+        const arg = args[i].replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+        script = script.replace(`{{${i + 1}}}`, arg);
       }
 
       return script;

diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts
@@ -1855,16 +1855,58 @@ describe('Cosmetic filters', () => {
   //   expect(CosmeticFilter.parse('###.selector /invalid/')).to.be.null;
   // });
 
-  it('#getScript', () => {
-    const parsed = CosmeticFilter.parse('foo.com##+js(script.js, arg1, arg2, arg3)');
-    expect(parsed).not.to.be.null;
-    if (parsed !== null) {
-      expect(parsed.getScript(new Map([['script.js', '{{1}},{{2}},{{3}}']]))).to.equal(
-        'arg1,arg2,arg3',
-      );
+  describe('#getScript', () => {
+    const simpleScriptlet = CosmeticFilter.parse('foo.com##+js(script.js, arg1, arg2, arg3)');
 
-      expect(parsed.getScript(new Map())).to.be.undefined;
-    }
+    it('returns undefined if script does not exist', () => {
+      expect(simpleScriptlet?.getScript(new Map())).to.be.undefined;
+    });
+
+    it('returns a script if one exists', () => {
+      expect(simpleScriptlet?.getScript(new Map([['script.js', 'test']]))).to.equal('test');
+    });
+
+    context('with arguments', () => {
+      it('inject values', () => {
+        expect(simpleScriptlet?.getScript(new Map([['script.js', '{{1}},{{2}},{{3}}']]))).to.equal(
+          'arg1,arg2,arg3',
+        );
+      });
+
+      it('escapes special characters', () => {
+        for (const character of [
+          '.',
+          '*',
+          '+',
+          '?',
+          '^',
+          '$',
+          '{',
+          '}',
+          '(',
+          ')',
+          '|',
+          '[',
+          ']',
+          '\\',
+        ]) {
+          const scriptlet = CosmeticFilter.parse(`foo.com##+js(script.js, ${character})`);
+          expect(scriptlet?.getScript(new Map([['script.js', '{{1}}']]))).to.equal(
+            `\\${character}`,
+          );
+        }
+      });
+
+      it('handles complex cases', () => {
+        for (const example of [
+          [String.raw`'\(a\)'`, String.raw`\\\(a\\\)`],
+          [String.raw`foo\*`, String.raw`foo\\\*`],
+        ]) {
+          const scriptlet = CosmeticFilter.parse(`foo.com##+js(script.js, ${example[0]})`);
+          expect(scriptlet?.getScript(new Map([['script.js', '{{1}}']]))).to.equal(example[1]);
+        }
+      });
+    });
   });
 
   describe('#getTokens', () => {
@@ -2361,4 +2403,27 @@ describe('scriptlets arguments parsing', () => {
       );
     }
   });
+
+  it('unescapes escaped scriptlets', () => {
+    for (const [scriptlet, expected] of [
+      [
+        String.raw`script-name, \u005C(a\u005C)`,
+        {
+          name: 'script-name',
+          args: [String.raw`\(a\)`],
+        },
+      ],
+      [
+        String.raw`script-name, {"a":1\u002C"b":2}`,
+        {
+          name: 'script-name',
+          args: [String.raw`{"a":1,"b":2}`],
+        },
+      ],
+    ] as const) {
+      expect(CosmeticFilter.parse(`foo.com##+js(${scriptlet})`)?.parseScript(), scriptlet).to.eql(
+        expected,
+      );
+    }
+  });
 });