Skip to content

Commit

Permalink
AG-28397 Fix issue with modifying RegExp.$1 value in `log-on-stack-…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
AdamWr committed Aug 26, 2024
1 parent aefd3da commit 8b4b2cc
Show file tree
Hide file tree
Showing 17 changed files with 232 additions and 1 deletion.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion src/helpers/match-stack.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
}

Expand All @@ -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);
};
59 changes: 59 additions & 0 deletions src/helpers/regexp-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
4 changes: 4 additions & 0 deletions src/scriptlets/abort-on-stack-trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
toRegExp,
isEmptyObject,
getNativeRegexpTest,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -170,4 +172,6 @@ abortOnStackTrace.injections = [
isEmptyObject,
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/evaldata-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
// following helpers are needed for helpers above
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -139,4 +141,6 @@ evalDataPrune.injections = [
// following helpers are needed for helpers above
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/json-prune-fetch-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
getWildcardPropertyInChain,
shouldAbortInlineOrInjectedScript,
getNativeRegexpTest,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/**
Expand Down Expand Up @@ -224,4 +226,6 @@ jsonPruneFetchResponse.injections = [
getWildcardPropertyInChain,
shouldAbortInlineOrInjectedScript,
getNativeRegexpTest,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/json-prune-xhr-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
getWildcardPropertyInChain,
shouldAbortInlineOrInjectedScript,
getNativeRegexpTest,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/**
Expand Down Expand Up @@ -365,4 +367,6 @@ jsonPruneXhrResponse.injections = [
getWildcardPropertyInChain,
shouldAbortInlineOrInjectedScript,
getNativeRegexpTest,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/json-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
toRegExp,
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -158,4 +160,6 @@ jsonPrune.injections = [
toRegExp,
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
backupRegExpValues,
restoreRegExpValues,
];
11 changes: 11 additions & 0 deletions src/scriptlets/log-on-stack-trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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 /, ''));
Expand Down Expand Up @@ -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;
};

Expand Down Expand Up @@ -120,4 +129,6 @@ logOnStacktrace.injections = [
hit,
logMessage,
isEmptyObject,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/set-constant.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {
getNativeRegexpTest,
setPropertyAccess,
toRegExp,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -456,4 +458,6 @@ setConstant.injections = [
getNativeRegexpTest,
setPropertyAccess,
toRegExp,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/trusted-prune-inbound-object.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
isEmptyObject,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -148,4 +150,6 @@ trustedPruneInboundObject.injections = [
getNativeRegexpTest,
shouldAbortInlineOrInjectedScript,
isEmptyObject,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/trusted-replace-outbound-text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
getNativeRegexpTest,
toRegExp,
isEmptyObject,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -295,4 +297,6 @@ trustedReplaceOutboundText.injections = [
getNativeRegexpTest,
toRegExp,
isEmptyObject,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/trusted-set-constant.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -278,4 +280,6 @@ trustedSetConstant.injections = [
// following helpers should be imported and injected
// because they are used by helpers above
shouldAbortInlineOrInjectedScript,
backupRegExpValues,
restoreRegExpValues,
];
4 changes: 4 additions & 0 deletions src/scriptlets/trusted-suppress-native-method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
isStringMatched,
isArrayMatched,
isObjectMatched,
backupRegExpValues,
restoreRegExpValues,
} from '../helpers/index';

/* eslint-disable max-len */
Expand Down Expand Up @@ -230,4 +232,6 @@ trustedSuppressNativeMethod.injections = [
isStringMatched,
isArrayMatched,
isObjectMatched,
backupRegExpValues,
restoreRegExpValues,
];
40 changes: 40 additions & 0 deletions tests/helpers/regexp-utils.spec.js
Original file line number Diff line number Diff line change
@@ -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');
});
38 changes: 38 additions & 0 deletions tests/scriptlets/abort-on-stack-trace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
Loading

0 comments on commit 8b4b2cc

Please sign in to comment.