AG-28397 Fix issue with modifying RegExp.$1 value in log-on-stack-trace and abort-on-stack-trace scriptlets. #384

Squashed commit of the following:

commit 0f6b357
Merge: 1ce19f2 aefd3da
Author: Adam Wróblewski <adam@adguard.com>
Date:   Thu Aug 22 13:44:45 2024 +0200

    Merge branch 'master' into fix/AG-28397

commit 1ce19f2
Author: Adam Wróblewski <adam@adguard.com>
Date:   Thu Aug 22 13:42:45 2024 +0200

    Fix changelog
    Rename getRegexpValues to backupRegExpValues
    Rename setPreviousRegExpValues to restoreRegExpValues
    Log error
    Add test for backupRegExpValues

commit 579b9d9
Author: Adam Wróblewski <adam@adguard.com>
Date:   Thu Aug 22 11:59:59 2024 +0200

    Change null to void

commit ecb91de
Author: Adam Wróblewski <adam@adguard.com>
Date:   Thu Aug 22 10:55:31 2024 +0200

    Fix issue with modifying `RegExp.$1` value in `log-on-stack-trace` and `abort-on-stack-trace` scriptlets

Author: AdamWr

CHANGELOG.md

@@ -18,7 +18,13 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic
 - new values to `set-local-storage-item` and `set-session-storage-item` scriptlets:
   `allowed`, `denied` [#445]
 
+### Fixed
+
+- issue with modyfing `RegExp.$1, …, RegExp.$9` values
+  in `log-on-stack-trace` and `abort-on-stack-trace` scriptlets [#384]
+
 [Unreleased]: https://github.com/AdguardTeam/Scriptlets/compare/v1.11.16...HEAD
+[#384]: https://github.com/AdguardTeam/Scriptlets/issues/384
 [#301]: https://github.com/AdguardTeam/Scriptlets/issues/301
 [#439]: https://github.com/AdguardTeam/Scriptlets/issues/439
 [#444]: https://github.com/AdguardTeam/Scriptlets/issues/444

src/helpers/match-stack.ts

@@ -1,6 +1,6 @@
 import { toRegExp } from './string-utils';
 import { shouldAbortInlineOrInjectedScript } from './script-source-utils';
-import { getNativeRegexpTest } from './regexp-utils';
+import { getNativeRegexpTest, backupRegExpValues, restoreRegExpValues } from './regexp-utils';
 
 /**
  * Checks if the stackTrace contains stackRegexp
@@ -15,7 +15,12 @@ export const matchStackTrace = (stackMatch: string | undefined, stackTrace: stri
         return true;
     }
 
+    const regExpValues = backupRegExpValues();
+
     if (shouldAbortInlineOrInjectedScript(stackMatch, stackTrace)) {
+        if (regExpValues.length && regExpValues[0] !== RegExp.$1) {
+            restoreRegExpValues(regExpValues);
+        }
         return true;
     }
 
@@ -26,5 +31,8 @@ export const matchStackTrace = (stackMatch: string | undefined, stackTrace: stri
         .map((line) => line.trim()) // trim the lines
         .join('\n');
 
+    if (regExpValues.length && regExpValues[0] !== RegExp.$1) {
+        restoreRegExpValues(regExpValues);
+    }
     return getNativeRegexpTest().call(stackRegexp, refinedStackTrace);
 };

src/helpers/regexp-utils.ts

@@ -12,3 +12,62 @@ export const getNativeRegexpTest = (): (string: string) => boolean => {
     }
     throw new Error('RegExp.prototype.test is not a function');
 };
+
+/**
+ * Retrieves the values of the global RegExp.$1, …, RegExp.$9 properties
+ * The problem is that RegExp.$1 is modified by scriptlet and according
+ * to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/n#description
+ * the values of $1, …, $9 update whenever a RegExp instance makes a successful match
+ * so we need to save these values and then change them back.
+ * Related issue - https://github.com/AdguardTeam/Scriptlets/issues/384
+ *
+ * @returns {Array} An array containing the values of the RegExp.$1, …, RegExp.$9 properties.
+ */
+export const backupRegExpValues = (): Array<any> => {
+    try {
+        const arrayOfRegexpValues = [];
+        for (let index = 1; index < 10; index += 1) {
+            const value = `$${index}`;
+            if (!(RegExp as any)[value]) {
+                break;
+            }
+            arrayOfRegexpValues.push((RegExp as any)[value]);
+        }
+        return arrayOfRegexpValues;
+    } catch (error) {
+        return [];
+    }
+};
+
+/**
+ * Sets previous values of the RegExp.$1, …, RegExp.$9 properties.
+ *
+ * @param {Array} array
+ * @returns {void}
+ */
+export const restoreRegExpValues = (array: Array<any>): void => {
+    if (!array.length) {
+        return;
+    }
+    try {
+        let stringPattern = '';
+        if (array.length === 1) {
+            stringPattern = `(${array[0]})`;
+        } else {
+            // Create a string pattern with a capturing group from passed array,
+            // e.g. ['foo', 'bar', 'baz'] will create '(foo),(bar),(baz)' string
+            stringPattern = array.reduce((accumulator, currentValue, currentIndex) => {
+                if (currentIndex === 1) {
+                    return `(${accumulator}),(${currentValue})`;
+                }
+                return `${accumulator},(${currentValue})`;
+            });
+        }
+        const regExpGroup = new RegExp(stringPattern);
+        array.toString().replace(regExpGroup, '');
+    } catch (error) {
+        const message = `Failed to restore RegExp values: ${error}`;
+        // eslint-disable-next-line no-console
+        console.log(message);
+    }
+};

src/scriptlets/abort-on-stack-trace.js

@@ -14,6 +14,8 @@ import {
     toRegExp,
     isEmptyObject,
     getNativeRegexpTest,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -170,4 +172,6 @@ abortOnStackTrace.injections = [
     isEmptyObject,
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/evaldata-prune.js

@@ -10,6 +10,8 @@ import {
     // following helpers are needed for helpers above
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -139,4 +141,6 @@ evalDataPrune.injections = [
     // following helpers are needed for helpers above
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/json-prune-fetch-response.ts

@@ -24,6 +24,8 @@ import {
     getWildcardPropertyInChain,
     shouldAbortInlineOrInjectedScript,
     getNativeRegexpTest,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /**
@@ -224,4 +226,6 @@ jsonPruneFetchResponse.injections = [
     getWildcardPropertyInChain,
     shouldAbortInlineOrInjectedScript,
     getNativeRegexpTest,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/json-prune-xhr-response.ts

@@ -22,6 +22,8 @@ import {
     getWildcardPropertyInChain,
     shouldAbortInlineOrInjectedScript,
     getNativeRegexpTest,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /**
@@ -365,4 +367,6 @@ jsonPruneXhrResponse.injections = [
     getWildcardPropertyInChain,
     shouldAbortInlineOrInjectedScript,
     getNativeRegexpTest,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/json-prune.js

@@ -10,6 +10,8 @@ import {
     toRegExp,
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -158,4 +160,6 @@ jsonPrune.injections = [
     toRegExp,
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/log-on-stack-trace.js

@@ -6,6 +6,8 @@ import {
     // following helpers should be imported and injected
     // because they are used by helpers above
     isEmptyObject,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -37,6 +39,8 @@ export function logOnStacktrace(source, property) {
     }
 
     const refineStackTrace = (stackString) => {
+        const regExpValues = backupRegExpValues();
+
         // Split stack trace string by lines and remove first two elements ('Error' and getter call)
         // Remove '    at ' at the start of each string
         const stackSteps = stackString.split('\n').slice(2).map((line) => line.replace(/ {4}at /, ''));
@@ -68,6 +72,11 @@ export function logOnStacktrace(source, property) {
             /* eslint-disable-next-line prefer-destructuring */
             logInfoObject[pair[0]] = pair[1];
         });
+
+        if (regExpValues.length && regExpValues[0] !== RegExp.$1) {
+            restoreRegExpValues(regExpValues);
+        }
+
         return logInfoObject;
     };
 
@@ -120,4 +129,6 @@ logOnStacktrace.injections = [
     hit,
     logMessage,
     isEmptyObject,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/set-constant.js

@@ -21,6 +21,8 @@ import {
     getNativeRegexpTest,
     setPropertyAccess,
     toRegExp,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -456,4 +458,6 @@ setConstant.injections = [
     getNativeRegexpTest,
     setPropertyAccess,
     toRegExp,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/trusted-prune-inbound-object.js

@@ -12,6 +12,8 @@ import {
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
     isEmptyObject,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -148,4 +150,6 @@ trustedPruneInboundObject.injections = [
     getNativeRegexpTest,
     shouldAbortInlineOrInjectedScript,
     isEmptyObject,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/trusted-replace-outbound-text.ts

@@ -9,6 +9,8 @@ import {
     getNativeRegexpTest,
     toRegExp,
     isEmptyObject,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -295,4 +297,6 @@ trustedReplaceOutboundText.injections = [
     getNativeRegexpTest,
     toRegExp,
     isEmptyObject,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/trusted-set-constant.js

@@ -21,6 +21,8 @@ import {
     // following helpers should be imported and injected
     // because they are used by helpers above
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -278,4 +280,6 @@ trustedSetConstant.injections = [
     // following helpers should be imported and injected
     // because they are used by helpers above
     shouldAbortInlineOrInjectedScript,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

src/scriptlets/trusted-suppress-native-method.ts

@@ -20,6 +20,8 @@ import {
     isStringMatched,
     isArrayMatched,
     isObjectMatched,
+    backupRegExpValues,
+    restoreRegExpValues,
 } from '../helpers/index';
 
 /* eslint-disable max-len */
@@ -230,4 +232,6 @@ trustedSuppressNativeMethod.injections = [
     isStringMatched,
     isArrayMatched,
     isObjectMatched,
+    backupRegExpValues,
+    restoreRegExpValues,
 ];

tests/helpers/regexp-utils.spec.js

@@ -0,0 +1,40 @@
+import { restoreRegExpValues, backupRegExpValues } from '../../src/helpers';
+
+test('restoreRegExpValues() check if correct value have been set', async () => {
+    restoreRegExpValues(['foo']);
+    expect(RegExp.$1).toBe('foo');
+    expect(RegExp.$2).toBe('');
+});
+
+test('restoreRegExpValues() check if correct values have been set', async () => {
+    restoreRegExpValues(['test', 'abc', 'xyz', 'aaaa', '123']);
+    expect(RegExp.$1).toBe('test');
+    expect(RegExp.$2).toBe('abc');
+    expect(RegExp.$3).toBe('xyz');
+    expect(RegExp.$4).toBe('aaaa');
+    expect(RegExp.$5).toBe('123');
+    expect(RegExp.$6).toBe('');
+});
+
+test('backupRegExpValues() and restoreRegExpValues(), modify values and restore them', async () => {
+    const regExp = /(\w+)\s(\w+)/;
+    const string = 'div a';
+    string.replace(regExp, '$2, $1');
+
+    expect(RegExp.$1).toBe('div');
+    expect(RegExp.$2).toBe('a');
+
+    const backupRegexp = backupRegExpValues();
+
+    const regExp2 = /(\w+)\s(\w+)/;
+    const string2 = 'qwerty zxcvbn';
+    string2.replace(regExp2, '$2, $1');
+
+    expect(RegExp.$1).toBe('qwerty');
+    expect(RegExp.$2).toBe('zxcvbn');
+
+    restoreRegExpValues(backupRegexp);
+
+    expect(RegExp.$1).toBe('div');
+    expect(RegExp.$2).toBe('a');
+});

tests/scriptlets/abort-on-stack-trace.test.js

@@ -492,3 +492,41 @@ test('abort RegExp, matches stack', (assert) => {
     );
     assert.strictEqual(window.hit, 'FIRED', 'hit fired');
 });
+
+test('abort document.createElement, matches stack', (assert) => {
+    const property = 'document.createElement';
+    const stackMatch = 'Object.createElemenTest';
+    const scriptletArgs = [property, stackMatch];
+    let testPassed = false;
+
+    runScriptlet(name, scriptletArgs);
+
+    function testCreateElement() {
+        const regExp = /(\w+)\s(\w+)/;
+        const string = 'div a';
+        const testString = string.replace(regExp, '$2, $1');
+        const div = document.createElement(RegExp.$1);
+        div.textContent = testString;
+        testPassed = true;
+    }
+
+    const matchTestCreateElement = {
+        createElemenTest: () => {
+            const regExp = /(\w+)\s(\w+)/;
+            const string = 'div a';
+            const testString = string.replace(regExp, '$2, $1');
+            const div = document.createElement(RegExp.$1);
+            div.textContent = testString;
+        },
+    };
+
+    testCreateElement();
+
+    assert.throws(
+        matchTestCreateElement.createElemenTest,
+        /ReferenceError/,
+        `Reference error thrown when trying to access property ${property}`,
+    );
+    assert.strictEqual(testPassed, true, 'testPassed set to true');
+    assert.strictEqual(window.hit, 'FIRED', 'hit fired');
+});