Firefox · Core
CVE-2025-8039
Logic Error in Core
Overview
Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifbrowser/components/urlbar/UrlbarInput.sys.mjs |
modified | |
ifbrowser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjs |
modified |
Files Changed
browser/components/urlbar/UrlbarInput.sys.mjsbrowser/components/urlbar/UrlbarSearchTermsPersistence.sys.mjsbrowser/components/urlbar/tests/browser/browser.tomlbrowser/components/urlbar/tests/browser/browser_UrlbarInput_searchTerms_uri_mismatch.jsbrowser/components/urlbar/tests/browser/head.js
Patch
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…
References
On This Page