From 8985376b0001d3a5e48c45c78dc265f2f830e616 Mon Sep 17 00:00:00 2001
From: Raymond Hill <rhill@raymondhill.net>
Date: Tue, 10 Nov 2020 10:43:26 -0500
Subject: [PATCH] Fix timing issue with cached redirection to web accessible
 resources

Reported internally by @gwarser.

In rare occasion, a timing issue could cause uBO to redirect
to a web accessible resource meant to be used for another
network request. This is a regression introduced with the
following commit:

- https://github.com/gorhill/uBlock/commit/2e5d32e96798dd55f3fae66d7091645ff7ad3784

Additionally, I identified another issue which would cause
cached redirection to fail when a cache entry with redirection
to a web accessible resource was being reused, an issue which
could especially affect pages which are generated dynamically
(i.e. without full page reload).
---
 platform/chromium/vapi-background.js |  2 +-
 src/js/messaging.js                  |  2 +-
 src/js/pagestore.js                  | 40 ++++++++++++++++------------
 src/js/redirect-engine.js            |  4 +--
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/platform/chromium/vapi-background.js b/platform/chromium/vapi-background.js
index a6ae9a288..e5f06c760 100644
--- a/platform/chromium/vapi-background.js
+++ b/platform/chromium/vapi-background.js
@@ -1169,7 +1169,7 @@ vAPI.warSecret = (( ) => {
         lastSecretTime = Date.now();
         const secret = generateSecret();
         secrets.push(secret);
-        return `?secret=${secret}`;
+        return secret;
     };
 })();
 
diff --git a/src/js/messaging.js b/src/js/messaging.js
index 253692f33..61d3716e6 100644
--- a/src/js/messaging.js
+++ b/src/js/messaging.js
@@ -748,7 +748,7 @@ const onMessage = function(request, sender, callback) {
             mouse: µb.epickerArgs.mouse,
             zap: µb.epickerArgs.zap,
             eprom: µb.epickerArgs.eprom,
-            pickerURL: vAPI.getURL(`/web_accessible_resources/epicker-ui.html${vAPI.warSecret()}`),
+            pickerURL: vAPI.getURL(`/web_accessible_resources/epicker-ui.html?secret=${vAPI.warSecret()}`),
         };
         µb.epickerArgs.target = '';
         break;
diff --git a/src/js/pagestore.js b/src/js/pagestore.js
index d9827a6b9..20198234e 100644
--- a/src/js/pagestore.js
+++ b/src/js/pagestore.js
@@ -134,11 +134,23 @@ const NetFilteringResultCache = class {
     }
 
     lookupResult(fctxt) {
-        return this.results.get(
+        const entry = this.results.get(
             fctxt.getDocHostname() + ' ' +
             fctxt.type + ' ' +
             fctxt.url
         );
+        if ( entry === undefined ) { return; }
+        // We need to use a new WAR secret if one is present since WAR secrets
+        // can only be used once.
+        if (
+            entry.redirectURL !== undefined &&
+            entry.redirectURL.startsWith(this.extensionOriginURL)
+        ) {
+            const redirectURL = new URL(entry.redirectURL);
+            redirectURL.searchParams.set('secret', vAPI.warSecret());
+            entry.redirectURL = redirectURL.href;
+        }
+        return entry;
     }
 
     lookupAllBlocked(hostname) {
@@ -158,6 +170,7 @@ const NetFilteringResultCache = class {
 };
 
 NetFilteringResultCache.prototype.shelfLife = 15000;
+NetFilteringResultCache.prototype.extensionOriginURL = vAPI.getURL('/');
 
 /******************************************************************************/
 
@@ -267,18 +280,6 @@ const PageStore = class {
         this.frames = new Map();
         this.setFrameURL(0, tabContext.rawURL);
 
-        // The current filtering context is cloned because:
-        // - We may be called with or without the current context having been
-        //   initialized.
-        // - If it has been initialized, we do not want to change the state
-        //   of the current context.
-        const fctxt = µb.logger.enabled
-            ? µb.filteringContext
-                .duplicate()
-                .fromTabId(tabId)
-                .setURL(tabContext.rawURL)
-            : undefined;
-
         // https://github.com/uBlockOrigin/uBlock-issues/issues/314
         const masterSwitch = tabContext.getNetFilteringSwitch();
 
@@ -292,10 +293,14 @@ const PageStore = class {
             µb.logger.enabled &&
             context === 'tabCommitted'
         ) {
-            fctxt.setRealm('cosmetic')
-                 .setType('dom')
-                 .setFilter(µb.sessionSwitches.toLogData())
-                 .toLogger();
+            µb.filteringContext
+                .duplicate()
+                .fromTabId(tabId)
+                .setURL(tabContext.rawURL)
+                .setRealm('cosmetic')
+                .setType('dom')
+                .setFilter(µb.sessionSwitches.toLogData())
+                .toLogger();
         }
 
         return this;
@@ -540,6 +545,7 @@ const PageStore = class {
 
     filterRequest(fctxt) {
         fctxt.filter = undefined;
+        fctxt.redirectURL = undefined;
 
         if ( this.getNetFilteringSwitch(fctxt) === false ) {
             return 0;
diff --git a/src/js/redirect-engine.js b/src/js/redirect-engine.js
index 6204189b9..95db7673d 100644
--- a/src/js/redirect-engine.js
+++ b/src/js/redirect-engine.js
@@ -212,7 +212,7 @@ const RedirectEntry = class {
             fctxt instanceof Object &&
             fctxt.type !== 'xmlhttprequest'
         ) {
-            let url = `${this.warURL}${vAPI.warSecret()}`;
+            let url = `${this.warURL}?secret=${vAPI.warSecret()}`;
             if ( this.params !== undefined ) {
                 for ( const name of this.params ) {
                     const value = fctxt[name];
@@ -489,7 +489,7 @@ RedirectEngine.prototype.loadBuiltinResources = function() {
         }
         fetches.push(
             µBlock.assets.fetch(
-                `/web_accessible_resources/${name}${vAPI.warSecret()}`,
+                `/web_accessible_resources/${name}?secret=${vAPI.warSecret()}`,
                 { responseType: details.data }
             ).then(
                 result => process(result)