Pull request 1165: AG-38557 remove injection of remotely hosted code

Squashed commit of the following: commit 7585d80 Author: Maxim Topciu <[email protected]> Date: Mon Jan 13 17:05:56 2025 +0200 AG-38557 update version in package.json commit 78fc53a Author: Maxim Topciu <[email protected]> Date: Mon Jan 13 17:04:41 2025 +0200 AG-38557 remove unused code commit 7f3e15b Author: Maxim Topciu <[email protected]> Date: Mon Jan 13 17:03:25 2025 +0200 AG-38557 fix changelog commit caf26c6 Author: Maxim Topciu <[email protected]> Date: Mon Jan 13 15:33:54 2025 +0200 AG-38557 update changelog commit 0b43669 Author: Maxim Topciu <[email protected]> Date: Mon Jan 13 15:32:04 2025 +0200 AG-38557 fix eslint errors commit 6889ddc Author: Maxim Topciu <[email protected]> Date: Mon Jan 13 14:21:51 2025 +0200 AG-38557 remove injection of remotely hosted code
AdguardTeam · Jan 13, 2025 · 27f1dd0 · 27f1dd0
1 parent 5164b8d
commit 27f1dd0
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 306 deletions.
diff --git a/packages/tswebextension/CHANGELOG.md b/packages/tswebextension/CHANGELOG.md
@@ -17,6 +17,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 [AdguardBrowserExtension#3002]: https://github.com/AdguardTeam/AdguardBrowserExtension/issues/3002
 
+## [2.4.0-alpha.9] - 2025-01-13
+
+### Removed
+
+- Injection of remotely hosted script rules.
+
+[2.4.0-alpha.9]: https://github.com/AdguardTeam/tsurlfilter/releases/tag/tswebextension-v2.4.0-alpha.9
+
 ## [2.4.0-alpha.8] - 2024-12-23
 
 ### Changed

diff --git a/packages/tswebextension/package.json b/packages/tswebextension/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@adguard/tswebextension",
-  "version": "2.4.0-alpha.8",
+  "version": "2.4.0-alpha.9",
   "description": "This is a TypeScript library that implements AdGuard's extension API",
   "main": "dist/index.js",
   "typings": "dist/types/src/lib/mv2/background/index.d.ts",

diff --git a/packages/tswebextension/src/lib/mv3/background/cosmetic-api.ts b/packages/tswebextension/src/lib/mv3/background/cosmetic-api.ts
@@ -5,7 +5,6 @@ import {
 } from '@adguard/tsurlfilter';
 import { CosmeticRuleType } from '@adguard/agtree';
 
-import { CUSTOM_FILTERS_START_ID, LF, USER_FILTER_ID } from '../../common/constants';
 import { appContext } from './app-context';
 import { engineApi } from './engine-api';
 import { tabsApi } from '../tabs/tabs-api';
@@ -18,7 +17,7 @@ import { defaultFilteringLog, FilteringEventType } from '../../common/filtering-
 import { getDomain } from '../../common/utils/url';
 import { type ContentType } from '../../common/request-type';
 import { nanoid } from '../nanoid';
-import { localScriptRulesService, type LocalScriptFunction } from './services/local-script-rules-service';
+import { localScriptRulesService } from './services/local-script-rules-service';
 
 export type ContentScriptCosmeticData = {
     /**
@@ -42,18 +41,9 @@ export type ContentScriptCosmeticData = {
  */
 type ScriptsAndScriptletsData = {
     /**
-     * Script text which is combiner from JS rules only from User rules and Custom filters
-     * since they are added manually by users. That's why they are considered "local".
+     * Script texts from JS rules.
      */
-    localScriptText: string,
-
-    /**
-     * Script functions combined from JS rules from filters which are pre-built into the extension.
-     * That's why they are considered "local".
-     *
-     * Should be executed by chrome scripting api.
-     */
-    localScriptFunctions: LocalScriptFunction[],
+    scriptTexts: string[],
 
     /**
      * List of scriptlet data objects. No need to separate them by type since they are all safe.
@@ -179,52 +169,34 @@ export class CosmeticApi extends CosmeticApiCommon {
         `;
     }
 
-    /**
-     * Checks whether the cosmetic (JS) rule is added manually by user —
-     * is it located in User rules or Custom filters.
-     *
-     * @param rule Rule to check.
-     *
-     * @returns True if rule is added manually by user.
-     */
-    private static isUserAddedRule(rule: CosmeticRule): boolean {
-        const filterListId = rule.getFilterListId();
-        return filterListId >= CUSTOM_FILTERS_START_ID || filterListId === USER_FILTER_ID;
-    }
-
     /**
      * It is possible to follow all places using this logic by searching JS_RULES_EXECUTION.
      *
      * This is STEP 3: All previously matched script rules are processed and filtered:
      * - JS rules from pre-built filters (previously collected, pre-built and passed to the engine)
-     *   are going to be executed as functions via chrome.scripting API;
-     * - JS rules manually added by users (from User rules and Custom filters)
-     *   are going to be executed as script text via script tag injection.
+     *   are going to be executed as functions via chrome.scripting API.
      */
     /**
      * Generates data for scriptlets and local scripts:
      * - functions for scriptlets,
-     * - functions for JS rules from pre-built filters,
-     * - script text for JS rules from User rules and Custom filters.
+     * - script texts for JS rules from pre-built filters.
      *
      * @param cosmeticResult Object containing cosmetic rules.
      *
-     * @returns An object with data for scriptlets and local scripts — script text and functions.
+     * @returns An object with data for scriptlets and script texts.
      */
     public static getScriptsAndScriptletsData(cosmeticResult: CosmeticResult): ScriptsAndScriptletsData {
         const rules = cosmeticResult.getScriptRules();
 
         if (rules.length === 0) {
             return {
-                localScriptText: '',
-                localScriptFunctions: [],
+                scriptTexts: [],
                 scriptletDataList: [],
             };
         }
 
-        const uniqueScriptFunctions = new Set<LocalScriptFunction>();
+        const uniqueScriptTexts = new Set<string>();
         const scriptletDataList = [];
-        const uniqueScriptStrings = new Set<string>();
 
         for (let i = 0; i < rules.length; i += 1) {
             const rule = rules[i];
@@ -233,36 +205,19 @@ export class CosmeticApi extends CosmeticApiCommon {
                 if (scriptletData) {
                     scriptletDataList.push(scriptletData);
                 }
-            } else if (CosmeticApi.isUserAddedRule(rule)) {
-                // JS rule is manually added by user locally in the extension — save its script text.
-                const scriptText = rule.getScript();
-                if (scriptText) {
-                    uniqueScriptStrings.add(scriptText.trim());
-                }
             } else {
                 // TODO: Optimize script injection by checking if common scripts (e.g., AG_)
                 //  are actually used in the rules. If not, avoid injecting them to reduce overhead.
 
-                // JS rule is pre-built into the extension — save its function.
-                const scriptFunction = localScriptRulesService.getLocalScriptFunction(rule);
-                if (scriptFunction) {
-                    uniqueScriptFunctions.add(scriptFunction);
+                const ruleScriptText = rule.getContent();
+                if (ruleScriptText) {
+                    uniqueScriptTexts.add(ruleScriptText);
                 }
             }
         }
 
-        let scriptText = '';
-        uniqueScriptStrings.forEach((script) => {
-            scriptText += script.endsWith(';')
-                ? `${script}${LF}`
-                : `${script};${LF}`;
-        });
-
-        const wrappedScriptText = CosmeticApi.wrapScriptText(scriptText);
-
         return {
-            localScriptText: wrappedScriptText,
-            localScriptFunctions: [...uniqueScriptFunctions],
+            scriptTexts: [...uniqueScriptTexts],
             scriptletDataList,
         };
     }
@@ -327,65 +282,48 @@ export class CosmeticApi extends CosmeticApiCommon {
             return;
         }
 
-        const localScriptFunctions = frameContext.preparedCosmeticResult?.localScriptFunctions;
+        const scriptTexts = frameContext.preparedCosmeticResult?.scriptTexts;
 
-        if (!localScriptFunctions || localScriptFunctions.length === 0) {
+        if (!scriptTexts || scriptTexts.length === 0) {
             return;
         }
 
         try {
-            await Promise.all(localScriptFunctions.map((scriptFunction) => {
+            await Promise.all(scriptTexts.map((scriptText) => {
                 /**
                  * It is possible to follow all places using this logic by searching JS_RULES_EXECUTION.
                  *
-                 * This is STEP 4.1: Apply JS rules from pre-built filters — via chrome.scripting API.
+                 * This is STEP 4.1: Selecting only local script functions which were pre-built into the extension.
+                 */
+
+                /**
+                 * Here we check if the script text is local to guarantee that we don't execute remote code.
                  */
+                const isLocal = localScriptRulesService.isLocal(scriptText);
+                if (!isLocal) {
+                    return;
+                }
+
+                /**
+                 * Here we get the function associated with the script text.
+                 */
+                const localScriptFunction = localScriptRulesService.getLocalScriptFunction(scriptText);
+                if (!localScriptFunction) {
+                    return;
+                }
+
+                // eslint-disable-next-line consistent-return
                 return ScriptingApi.executeScriptFunc({
                     tabId,
                     frameId,
-                    scriptFunction,
+                    scriptFunction: localScriptFunction,
                 });
             }));
         } catch (e) {
             logger.debug('[applyJsFuncsByTabAndFrame] error occurred during injection', getErrorMessage(e));
         }
     }
 
-    /**
-     * Injects js locally added rules by user to specified tab and frame.
-     *
-     * @param tabId Tab id.
-     * @param frameId Frame id.
-     */
-    public static async applyJsTextByTabAndFrame(tabId: number, frameId: number): Promise<void> {
-        const frameContext = tabsApi.getFrameContext(tabId, frameId);
-
-        if (!frameContext) {
-            return;
-        }
-
-        const localScriptText = frameContext.preparedCosmeticResult?.localScriptText;
-
-        if (!localScriptText) {
-            return;
-        }
-
-        try {
-            /**
-             * It is possible to follow all places using this logic by searching JS_RULES_EXECUTION.
-             *
-             * This is STEP 4.2: Apply JS rules manually added by users — via script tag injection.
-             */
-            await ScriptingApi.executeScriptText({
-                tabId,
-                frameId,
-                scriptText: localScriptText,
-            });
-        } catch (e) {
-            logger.debug('[applyJsTextByTabAndFrame] error occurred during injection', getErrorMessage(e));
-        }
-    }
-
     /**
      * Injects js to specified tab and frame.
      *

diff --git a/packages/tswebextension/src/lib/mv3/background/cosmetic-frame-processor.ts b/packages/tswebextension/src/lib/mv3/background/cosmetic-frame-processor.ts
@@ -209,8 +209,7 @@ export class CosmeticFrameProcessor {
         const cosmeticResult = engineApi.getCosmeticResult(url, result.getCosmeticOption());
 
         const {
-            localScriptText,
-            localScriptFunctions,
+            scriptTexts,
             scriptletDataList,
         } = CosmeticApi.getScriptsAndScriptletsData(cosmeticResult);
 
@@ -221,8 +220,7 @@ export class CosmeticFrameProcessor {
             matchingResult: result,
             cosmeticResult,
             preparedCosmeticResult: {
-                localScriptText,
-                localScriptFunctions,
+                scriptTexts,
                 scriptletDataList,
                 cssText,
             },
@@ -266,8 +264,7 @@ export class CosmeticFrameProcessor {
         const cosmeticResult = engineApi.getCosmeticResult(url, result.getCosmeticOption());
 
         const {
-            localScriptText,
-            localScriptFunctions,
+            scriptTexts,
             scriptletDataList,
         } = CosmeticApi.getScriptsAndScriptletsData(cosmeticResult);
 
@@ -277,8 +274,7 @@ export class CosmeticFrameProcessor {
             matchingResult: result,
             cosmeticResult,
             preparedCosmeticResult: {
-                localScriptText,
-                localScriptFunctions,
+                scriptTexts,
                 scriptletDataList,
                 cssText,
             },