AG-24613: prevent csp_report requests if they are third-party or cont…

…ains extension id Merge in ADGUARD-FILTERS/tsurlfilter from fix/AG-24613 to master Squashed commit of the following: commit 06d14a8 Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 20:35:02 2023 +0400 fixed changelogs commit bfa281e Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 20:34:13 2023 +0400 fixed changelogs commit a0777a9 Author: Maxim Topciu <[email protected]> Date: Thu Aug 17 19:27:10 2023 +0300 AG-24613 fix todo comment commit 4d0f131 Merge: 4ae2ae0 8a3a1e8 Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 19:55:35 2023 +0400 Merge branch 'master' into fix/AG-24613 commit 4ae2ae0 Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 19:54:13 2023 +0400 added changelogs commit 7a53a05 Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 19:06:28 2023 +0400 fixed todos commit 35786ad Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 19:04:48 2023 +0400 removed todo commit 7fa6dbb Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 18:50:56 2023 +0400 remove deprecated check commit 0d39568 Author: Dmitriy Seregin <[email protected]> Date: Thu Aug 17 18:32:14 2023 +0400 fixed logging commit 1e462fe Author: Dmitriy Seregin <[email protected]> Date: Wed Aug 16 19:03:17 2023 +0400 fix broken tests commit 1a134e9 Author: Dmitriy Seregin <[email protected]> Date: Wed Aug 16 18:42:57 2023 +0400 AG-24613: prevent csp_report requests if they are third-party or contains extension id
AdguardTeam · Aug 17, 2023 · 7ff682c · 7ff682c
1 parent 8a3a1e8
commit 7ff682c
Show file tree

Hide file tree

Showing 11 changed files with 129 additions and 9 deletions.
diff --git a/packages/tsurlfilter/CHANGELOG.md b/packages/tsurlfilter/CHANGELOG.md
@@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 <!-- TODO: manually add compare links for version to the end of the file -->
 <!-- e.g. [1.0.77]: https://github.com/AdguardTeam/tsurlfilter/compare/tsurlfilter-v1.0.76...tsurlfilter-v1.0.77 -->
 
+## [Unreleased]
+
+### Added
+- Added `CspReport` to RequestType enum.
+
 ## [2.1.8] - 2023-08-14
 
 ### Fixed

diff --git a/packages/tsurlfilter/src/request-type.ts b/packages/tsurlfilter/src/request-type.ts
@@ -28,8 +28,10 @@ export const RequestType = {
     WebSocket: 512, // 1 << 9
     /** (navigator.sendBeacon()) $ping */
     Ping: 1024, // 1 << 10
+    /** csp_report */
+    CspReport: 2048, // 1 << 11
     /** any other request type */
-    Other: 2048, // 1 << 11
+    Other: 4096, // 1 << 12
 } as const;
 
 // intentionally naming the variable the same as the type

diff --git a/packages/tsurlfilter/src/rules/declarative-converter/declarative-rule.ts b/packages/tsurlfilter/src/rules/declarative-converter/declarative-rule.ts
@@ -36,6 +36,7 @@ export enum ResourceType {
     WebSocket = 'websocket',
     Other = 'other',
     // Temporary not using
+    // TODO: Add csp_report handler similar to AG-24613 but in declarative way.
     // CspReport = 'csp_report',
     // WebTransport = 'webtransport',
     // WebBundle = 'webbundle',

diff --git a/packages/tsurlfilter/test/rules/declarative-converter/declarative-rule-converter.test.ts b/packages/tsurlfilter/test/rules/declarative-converter/declarative-rule-converter.test.ts
@@ -858,7 +858,7 @@ describe('DeclarativeRuleConverter', () => {
         expect(declarativeRules).toHaveLength(2);
         expect(declarativeRules[0]).toStrictEqual({
             id: 1,
-            priority: 56,
+            priority: 55,
             action: {
                 type: 'block',
             },

diff --git a/packages/tswebextension/CHANGELOG.md b/packages/tswebextension/CHANGELOG.md
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+- Blocking third-party requests with `csp_report` content-type.
+
 ### Fixed
 - Do not expose JS rules in global page scope
 

diff --git a/packages/tswebextension/src/lib/common/filtering-log.ts b/packages/tswebextension/src/lib/common/filtering-log.ts
@@ -11,7 +11,6 @@ export enum FilteringEventType {
     TabReload = 'tabReload',
     ApplyBasicRule = 'applyBasicRule',
     ApplyCosmeticRule = 'applyCosmeticRule',
-    // TODO: Doesn't look like it's being used.
     ApplyCspRule = 'applyCspRule',
     ReceiveResponse = 'receiveResponse',
     Cookie = 'cookie',
@@ -22,6 +21,7 @@ export enum FilteringEventType {
     ContentFilteringFinish = 'contentFilteringFinish',
     StealthAction = 'stealthAction',
     JsInject = 'jsInject',
+    CspReportBlocked = 'cspReportBlocked',
 }
 
 /**
@@ -317,6 +317,29 @@ export type JsInjectEvent = {
     data: JsInjectEventData;
 };
 
+/**
+ * {@link CspReportBlockedEvent} Event data.
+ */
+export type CspReportBlockedEventData = {
+    tabId: number;
+    eventId: string;
+    cspReportBlocked: boolean;
+    requestUrl: string,
+    requestType: ContentType,
+    timestamp: number,
+    requestThirdParty: boolean,
+    method: string,
+};
+
+/**
+ * Dispatched by manifest v2 WebRequestApi.onBeforeCspReport handler when
+ * csp_report blocked in onBeforeRequest event handler.
+ */
+export type CspReportBlockedEvent = {
+    type: FilteringEventType.CspReportBlocked
+    data: CspReportBlockedEventData;
+};
+
 /**
  * Filtering events union.
  *
@@ -337,7 +360,8 @@ export type FilteringLogEvent =
     | ApplyCspRuleEvent
     | ApplyCosmeticRuleEvent
     | ReceiveResponseEvent
-    | JsInjectEvent;
+    | JsInjectEvent
+    | CspReportBlockedEvent;
 
 /**
  * Utility type for mapping {@link FilteringEventType} with specified {@link FilteringLogEvent}.

diff --git a/packages/tswebextension/src/lib/common/request-type.ts b/packages/tswebextension/src/lib/common/request-type.ts
@@ -99,7 +99,7 @@ export function getRequestType(resourceType: WebRequest.ResourceType): RequestTy
         case 'csp_report':
             return {
                 contentType: ContentType.CspReport,
-                requestType: RequestType.Other,
+                requestType: RequestType.CspReport,
             };
         default:
             return {

diff --git a/packages/tswebextension/src/lib/mv2/background/engine-api.ts b/packages/tswebextension/src/lib/mv2/background/engine-api.ts
@@ -45,6 +45,9 @@ export class EngineApi {
     /**
      * Gets app filtering status.
      *
+     * TODO: This method is duplicated in {@link EngineApi}. Consider moving it to {@link appContext}
+     *  itself (DRY). But appContext supposed to be deleted (v.zhelvis).
+     *
      * @returns True if filtering is enabled, otherwise returns false.
      */
     public get isFilteringEnabled(): boolean {
@@ -97,9 +100,9 @@ export class EngineApi {
             lists.push(new StringRuleList(USER_FILTER_ID, convertedUserRules));
         }
 
-        const allowlistRules = this.allowlist.getAllowlistRules();
-        if (allowlistRules) {
-            lists.push(allowlistRules);
+        const allowlistRulesList = this.allowlist.getAllowlistRules();
+        if (allowlistRulesList) {
+            lists.push(allowlistRulesList);
         }
 
         const stealthModeList = this.stealthApi.getStealthModeRuleList();

diff --git a/packages/tswebextension/src/lib/mv2/background/stealth-api.ts b/packages/tswebextension/src/lib/mv2/background/stealth-api.ts
@@ -64,6 +64,9 @@ export class StealthApi {
     /**
      * Gets app filtering status.
      *
+     * TODO: This method is duplicated in {@link EngineApi}. Consider moving it to {@link appContext}
+     *  itself (DRY). But appContext supposed to be deleted (v.zhelvis).
+     *
      * @returns True if filtering is enabled, otherwise returns false.
      */
     private get isFilteringEnabled(): boolean {

diff --git a/packages/tswebextension/src/lib/mv2/background/web-request-api.ts b/packages/tswebextension/src/lib/mv2/background/web-request-api.ts
@@ -212,6 +212,7 @@ export class WebRequestApi {
      */
     public static start(): void {
         // browser.webRequest Events
+        RequestEvents.onBeforeRequest.addListener(WebRequestApi.onBeforeCspReport);
         RequestEvents.onBeforeRequest.addListener(WebRequestApi.onBeforeRequest);
         RequestEvents.onBeforeSendHeaders.addListener(WebRequestApi.onBeforeSendHeaders);
         RequestEvents.onHeadersReceived.addListener(WebRequestApi.onHeadersReceived);
@@ -228,6 +229,7 @@ export class WebRequestApi {
      * Removes web request event handlers.
      */
     public static stop(): void {
+        RequestEvents.onBeforeRequest.removeListener(WebRequestApi.onBeforeCspReport);
         RequestEvents.onBeforeRequest.removeListener(WebRequestApi.onBeforeRequest);
         RequestEvents.onBeforeSendHeaders.removeListener(WebRequestApi.onBeforeSendHeaders);
         RequestEvents.onHeadersReceived.removeListener(WebRequestApi.onHeadersReceived);
@@ -757,4 +759,81 @@ export class WebRequestApi {
             })
             .catch(logger.debug);
     }
+
+    /**
+     * Intercepts csp_report requests.
+     * Check the URL of the report.
+     * For chromium and firefox:
+     * If it's sent to a third party, block it right away.
+     * For firefox only:
+     * If it contains moz://extension with our extension ID, block it as well.
+     * @see https://github.com/AdguardTeam/AdguardBrowserExtension/issues/1792.
+     *
+     * @param details Request details.
+     * @param details.context Request context.
+     *
+     * @returns Web request response or void if there is nothing to do.
+     */
+    private static onBeforeCspReport(
+        { context }: RequestData<WebRequest.OnBeforeRequestDetailsType>,
+    ): WebRequestEventResponse {
+        // If filtering is disabled - skip process request.
+        if (!engineApi.isFilteringEnabled) {
+            return undefined;
+        }
+
+        if (!context) {
+            return undefined;
+        }
+
+        const {
+            requestUrl,
+            requestType,
+            matchingResult,
+            tabId,
+            eventId,
+            timestamp,
+            thirdParty,
+            contentType,
+            method,
+        } = context;
+
+        /**
+         * Checks request type here instead of creating two event listener with
+         * different filter types via {@link RequestEvent.init} to simplify
+         * event handlers flow and create only one {@link RequestEvents.onBeforeRequest}
+         * listener and two WebRequest listeners: {@link WebRequestApi.onBeforeCspReport}
+         * and {@link WebRequestApi.onBeforeRequest}.
+         */
+        if (requestType !== RequestType.CspReport) {
+            return undefined;
+        }
+
+        // If filtering disabled for this request.
+        if (matchingResult?.getBasicResult()?.isFilteringDisabled()) {
+            return undefined;
+        }
+
+        if (thirdParty) {
+            defaultFilteringLog.publishEvent({
+                type: FilteringEventType.CspReportBlocked,
+                data: {
+                    tabId,
+                    eventId,
+                    cspReportBlocked: true,
+                    requestUrl,
+                    requestType: contentType,
+                    timestamp,
+                    requestThirdParty: thirdParty,
+                    method,
+                },
+            });
+
+            return { cancel: true };
+        }
+
+        // Don't check for moz://extension because it was fixed in
+        // https://bugzilla.mozilla.org/show_bug.cgi?id=1588957#c10.
+        return undefined;
+    }
 }
diff --git a/packages/tswebextension/test/lib/mv2/background/request/request-type.test.ts b/packages/tswebextension/test/lib/mv2/background/request/request-type.test.ts
@@ -60,7 +60,7 @@ describe('Request Type', () => {
             },
             csp_report: {
                 contentType: ContentType.CspReport,
-                requestType: RequestType.Other,
+                requestType: RequestType.CspReport,
             },
             xmlhttprequest: {
                 contentType: ContentType.XmlHttpRequest,