Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionIn some cases search terms persisted in the URL bar even after navigating away from the search page.
ComponentCore
Bug ClassLogic Error
Tracker1970997
Fix commita34b558392eb (firefox) +201/-51
CISA KEVNot listed
CreditedSören Hentzschel
Disclosed2025-07-22

Changed Functions

FunctionChangeNotes
if
browser/components/urlbar/UrlbarInput.sys.mjs
modified
if
browser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs
modified

Files Changed

  • browser/components/urlbar/UrlbarInput.sys.mjs
  • browser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs
  • browser/components/urlbar/tests/browser/browser.toml
  • browser/components/urlbar/tests/browser/browser_UrlbarInput_searchTerms_uri_mismatch.js
  • browser/components/urlbar/tests/browser/head.js
diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs
index 79da060bef4..5cef1147cea 100644
--- a/browser/components/urlbar/UrlbarInput.sys.mjs
+++ b/browser/components/urlbar/UrlbarInput.sys.mjs
@@ -464,54 +464,13 @@ export class UrlbarInput {
     }
 
     let state = this.getBrowserState(this.window.gBrowser.selectedBrowser);
-    if (lazy.UrlbarPrefs.isPersistedSearchTermsEnabled()) {
-      // The first time the browser URI has been loaded to the input. If
-      // persist is not defined, it is likely due to the tab being created in
-      // the background or an existing tab moved to a new window and we have to
-      // do the work for the first time.
-      let firstView = (!isSameDocument && !dueToTabSwitch) || !state.persist;
-      if (firstView) {
-        lazy.UrlbarSearchTermsPersistence.setPersistenceState(
-          state,
-          this.window.gBrowser.selectedBrowser.originalURI
-        );
-      }
-      let shouldPersist =
-        !hideSearchTerms &&
-        lazy.UrlbarSearchTermsPersistence.shouldPersist(state, {
-          dueToTabSwitch,
-          isSameDocument,
-          uri,
-          userTypedValue: this.window.gBrowser.userTypedValue,
-          firstView,
-        });
-
-      // When persisting, userTypedValue should have a value consistent with the
-      // search terms to mimic a user typing the search terms.
-      // When turning off persist, check if the userTypedValue needs to be
-      // removed in order for the URL to return to the address bar. Single page
-      // application SERPs will load secondary search pages (e.g. Maps, Images)
-      // with the same document, which won't unset userTypedValue.
-      if (shouldPersist) {
-        this.window.gBrowser.userTypedValue = state.persist.searchTerms;
-      } else if (
-        isSameDocument &&
-        state.persist.shouldPersist &&
-        !shouldPersist
-      ) {
-        this.window.gBrowser.userTypedValue = null;
-      }
-      state.persist.shouldPersist = shouldPersist;
-      this.toggleAttribute("persistsearchterms", state.persist.shouldPersist);
-      if (state.persist.shouldPersist && !isSameDocument) {
-        Glean.urlbarPersistedsearchterms.viewCount.add(1);
-      }
-    } else if (state.persist) {
-      // Ensure the persist search state is unloaded for tabs that had state
-      // related to Persisted Search but disabled the feature.
-      this.removeAttribute("persistsearchterms");
-      delete state.persist;
-    }
+    this.#handlePersistedSearchTerms({
+      state,
+      uri,
+      dueToTabSwitch,
+      hideSearchTerms,
+      isSameDocument,
+    });
 
     let value = this.window.gBrowser.userTypedValue;
     let valid = false;
@@ -3901,6 +3860,86 @@ export class UrlbarInput {
     this.searchModeSwitcher?.onSearchModeChanged();
   }
 
+  /**
+   * Handles persisted search terms logic for the current browser. This manages
+   * state and updates the UI accordingly.
+   *
+   * @param {object} options
+   * @param {object} options.state
+   *   The state object for the currently viewed browser.
+   * @param {boolean} options.hideSearchTerms
+   *   True if we must hide the search terms and instead show the page URL.
+   * @param {boolean} options.dueToTabSwitch
+   *   True if the browser was revealed again due to a tab switch.
+   * @param {boolean} options.isSameDocument
+   *   True if the page load was same document.
+   * @param {nsIURI} [options.uri]
+   *   The latest URI of the page.
+   * @returns {boolean}
+   *   Whether search terms should persist.
+   */
+  #handlePersistedSearchTerms({
+    state,
+    hideSearchTerms,
+    dueToTabSwitch,
+    isSameDocument,
+    uri,
+  }) {
+    if (!lazy.UrlbarPrefs.isPersistedSearchTermsEnabled()) {
+      if (state.persist) {
+        this.removeAttribute("persistsearchterms");
+        delete state.persist;
+      }
+      return false;
+    }
+
+    // The first time the browser URI has been loaded to the input. If
+    // persist is not defined, it is likely due to the tab being created in
+    // the background or an existing tab moved to a new window and we have to
+    // do the work for the first time.
+    let firstView = (!isSameDocument && !dueToTabSwitch) || !state.persist;
+
+    // Capture the shouldPersist property if it exists before
+    // setPersistenceState potentially modifies it.
+    let wasPersisting = state.persist?.shouldPersist ?? false;
+
+    if (firstView) {
+      lazy.UrlbarSearchTermsPersistence.setPersistenceState(
+        state,
+        this.window.gBrowser.selectedBrowser.originalURI
+      );
+    }
+    let shouldPersist =
+      !hideSearchTerms &&
+      lazy.UrlbarSearchTermsPersistence.shouldPersist(state, {
+        dueToTabSwitch,
+        isSameDocument,
+        uri: uri ?? this.window.gBrowser.currentURI,
+        userTypedValue: this.window.gBrowser.userTypedValue,
+        firstView,
+      });
+    // When persisting, userTypedValue should have a value consistent with the
+    // search terms to mimic a user typing the search terms.
+    // When turning off persist, check if the userTypedValue needs to be
+    // removed in order for the URL to return to the address bar. Single page
+    // application SERPs will load secondary search pages (e.g. Maps, Images)
+    // with the same document, which won't unset userTypedValue.
+    if (shouldPersist) {
+      this.window.gBrowser.userTypedValue = state.persist.searchTerms;
+    } else if (wasPersisting && !shouldPersist) {
+      this.window.gBrowser.userTypedValue = null;
+    }
+
+    state.persist.shouldPersist = shouldPersist;
+    this.toggleAttribute("persistsearchterms", state.persist.shouldPersist);
+
+    if (state.persist.shouldPersist && !isSameDocument) {
+      Glean.urlbarPersistedsearchterms.viewCount.add(1);
+    }
+
+    return shouldPersist;
+  }
+
   /**
    * Initializes the urlbar placeholder to the pre-saved engine name. We do this
    * via a preference, to avoid needing to synchronously init the search service.
diff --git a/browser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs b/browser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs
index f061752eaf9..0df0e233f57 100644
--- a/browser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs
+++ b/browser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs
@@ -242,6 +242,20 @@ class _UrlbarSearchTermsPersistence {
       return false;
     }
 
+    let origin;
+    try {
+      origin = URL.fromURI(uri)?.origin;
+    } catch (ex) {
+      return false;
+    }
+
+    // Bug 1972464: Prevent search terms from persisting across different origin
+    // due to a possible race condition. This check prevents cross-origin
+    // persistence until the persistence logic is refactored.
+    if (origin !== state.persist.origin) {
+      return false;
+    }
+
     return true;
   }
 
@@ -251,6 +265,10 @@ class _UrlbarSearchTermsPersistence {
       // Whether the engine that loaded the URI is the default search engine.
       isDefaultEngine: null,
 
+      // Temporary until we resolve Bug 1972464: Cache origin for validation
+      // checks. This should be removed once the architecture is refactored.
+      origin: null,
+
       // The name of the engine that was used to load the URI.
       originalEngineName: null,
 
@@ -260,16 +278,28 @@ class _UrlbarSearchTermsPersistence {
       provider: null,
 
       // The search string within the URI.
-      searchTerms: this.getSearchTerm(uri),
+      searchTerms: "",
 
       // Whether the search terms should persist.
       shouldPersist: null,
     };
 
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/browser/components/urlbar/tests/browser/browser.toml b/browser/components/urlbar/tests/browser/browser.toml
index ddf69813361..64732f385d1 100644
--- a/browser/components/urlbar/tests/browser/browser.toml
+++ b/browser/components/urlbar/tests/browser/browser.toml
@@ -92,6 +92,9 @@ tags = "persisted-search"
 ["browser_UrlbarInput_searchTerms_telemetry.js"]
 tags = "persisted-search"
 
+["browser_UrlbarInput_searchTerms_uri_mismatch.js"]
+tags = "persisted-search"
+
 ["browser_UrlbarInput_setURI.js"]
 https_first_disabled = true
 skip-if = ["os == 'mac' && os_version == '10.15' && processor == 'x86_64' && debug"] # Bug 1773790
diff --git a/browser/components/urlbar/tests/browser/browser_UrlbarInput_searchTerms_uri_mismatch.js b/browser/components/urlbar/tests/browser/browser_UrlbarInput_searchTerms_uri_mismatch.js
new file mode 100644
index 00000000000..d23f33c9758
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_UrlbarInput_searchTerms_uri_mismatch.js
@@ -0,0 +1,78 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Tests that search terms don't persist with navigating to non-SERP URIs.
+// Normally, when setURI is called, dependent properties for persisted search
+// like originalURI are set to a different value and the userTypedValue is
+// nullified. But, we should ensure that if setURI receives a URI with a
+// different origin from the originalURI, search terms will not persist.
+
+const SEARCH_STRING = "chocolate cake";
+
+add_setup(async function () {
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.showSearchTerms.featureGate", true]],
+  });
+  let cleanup = await installPersistTestEngines();
+  registerCleanupFunction(async function () {
+    await PlacesUtils.history.clear();
+    cleanup();
+  });
+});
+
+add_task(async function test_search_terms_cleared_on_non_serp_host() {
+  let { tab } = await searchWithTab(SEARCH_STRING);
+
+  let nonSerpUri = Services.io.newURI("https://www.foo.com/");
+  let originalURI = tab.linkedBrowser.originalURI;
+
+  Assert.equal(
+    originalURI.scheme,
+    nonSerpUri.scheme,
+    "Test URIs should have the same scheme."
+  );
+
+  Assert.notEqual(
+    originalURI.host,
+    nonSerpUri.host,
+    "Test URIs should have a different host."
+  );
+
+  gURLBar.setURI(nonSerpUri);
+
+  Assert.ok(
+    !gURLBar.hasAttribute("persistsearchterms"),
+    "Should not persist when setURI is called with a different host."
+  );
+
+  BrowserTestUtils.removeTab(tab);
+});
+
+add_task(async function test_search_terms_cleared_on_non_serp_scheme() {
+  let { tab } = await searchWithTab(SEARCH_STRING);
+
+  let nonSerpUri = Services.io.newURI("foo://www.example.com/");
+  let originalURI = tab.linkedBrowser.originalURI;
+
+  Assert.notEqual(
+    originalURI.scheme,
+    nonSerpUri.scheme,
+    "Test URIs should have a different scheme."
+  );
+
+  Assert.equal(
+    originalURI.host,
+    nonSerpUri.host,
+    "Test URIs should have the same host."
+  );
+
+  gURLBar.setURI(nonSerpUri);
+
+  Assert.ok(
+    !gURLBar.hasAttribute("persistsearchterms"),
+    "Should not persist when setURI is called with a different scheme."
+  );
+
+  BrowserTestUtils.removeTab(tab);
+});
diff --git a/browser/components/urlbar/tests/browser/head.js b/browser/components/urlbar/tests/browser/head.js
index 94673d8b23b..c2cd2b3b707 100644
--- a/browser/components/urlbar/tests/browser/head.js
+++ b/browser/components/urlbar/tests/browser/head.js
@@ -326,7 +326,7 @@ function assertSearchStringIsInUrlbar(
   );
   let state = win.gURLBar.getBrowserState(win.gBrowser.selectedBrowser);
   Assert.equal(
-    state.persist.searchTerms,
+    state.persist?.searchTerms,
     searchString,
     `Search terms should match.`
   );
Loading diff…