Firefox · Core
CVE-2026-100804
Logic Error in Core
Overview
Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
formodules/libpref/Preferences.cpp |
modified | |
ifmodules/libpref/Preferences.cpp |
modified |
Files Changed
modules/libpref/Preferences.cppmodules/libpref/Preferences.hmodules/libpref/test/gtest/Basics.cpp
Patch
diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp
index 7fe64921944..aeb7e7bc9ce 100644
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -1421,6 +1421,21 @@ static nsCString CopyStrippingTrailingDot(const nsACString& aDomain) {
return nsCString(aDomain);
}
+// A single trailing dot is normalized away; more than one is rejected since
+// only one would be stripped, leaving an empty trailing segment.
+static bool DomainEndsWithMultipleDots(const nsACString& aDomain) {
+ return StringEndsWith(aDomain, ".."_ns);
+}
+
+static bool DomainEndsWithMultipleDots(const char* const* aDomains) {
+ for (const char* const* p = aDomains; *p; ++p) {
+ if (DomainEndsWithMultipleDots(nsDependentCString(*p))) {
+ return true;
+ }
+ }
+ return false;
+}
+
// The fire-time payload of a pref-change callback: the function and its
// closure. Shared base of the trie's refcounted CallbackNode and the mirror
// list's inline entries (see MirrorCallbackList), so the notify, sweep and
@@ -3484,30 +3499,37 @@ nsPrefBranch::AddObserverImpl(const nsACString& aDomain, nsIObserver* aObserver,
mozilla::UniquePtr<PrefCallback> existing;
mObservers.Remove(&weakKey, &existing);
if (existing) {
- Preferences::UnregisterCallback(NotifyObserver, prefName,
- existing.get(),
- /* aPrefixMatch */ true);
+ nsresult rv = Preferences::UnregisterCallback(NotifyObserver, prefName,
+ existing.get(),
+ /* aPrefixMatch */ true);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ // The trie still points at the weak PrefCallback; keep it alive and
+ // leave the weak registration in place.
+ mObservers.InsertOrUpdate(&weakKey, std::move(existing));
+ return rv;
+ }
}
}
}
+ nsresult rv = NS_OK;
mObservers.WithEntryHandle(pCallback.get(), [&](auto&& p) {
if (p) {
NS_WARNING(
nsPrintfCString("Ignoring duplicate observer: %s", prefName.get())
.get());
} else {
- // We must pass a fully qualified preference name to the callback
- // aDomain == nullptr is the only possible failure, and we trapped it with
- // NS_ENSURE_ARG above.
- Preferences::RegisterCallback(NotifyObserver, prefName, pCallback.get(),
- /* aPrefixMatch */ true);
-
- p.Insert(std::move(pCallback));
+ // We must pass a fully qualified preference name to the callback.
+ rv = Preferences::RegisterCallback(NotifyObserver, prefName,
+ pCallback.get(),
+ /* aPrefixMatch */ true);
+ if (NS_SUCCEEDED(rv)) {
+ p.Insert(std::move(pCallback));
+ }
}
});
- return NS_OK;
+ return rv;
}
NS_IMETHODIMP
@@ -3529,8 +3551,9 @@ nsPrefBranch::RemoveObserverImpl(const nsACString& aDomain,
}
// Remove the relevant PrefCallback from mObservers and get an owning pointer
- // to it. Unregister the callback first, and then let the owning pointer go
- // out of scope and destroy the callback.
+ // to it. If unregistering the trie callback fails (e.g. because mObservers
+ // and the trie have drifted out of sync), the CallbackNode in the trie still
+ // holds Data() == this PrefCallback, so put it back.
const nsCString& prefName = GetPrefName(aDomain);
PrefCallback key(prefName, aObserver, this);
mozilla::UniquePtr<PrefCallback> pCallback;
@@ -3551,6 +3574,10 @@ nsPrefBranch::RemoveObserverImpl(const nsACString& aDomain,
rv = Preferences::UnregisterCallback(NotifyObserver, prefName,
pCallback.get(),
/* aPrefixMatch */ true);
+ if (NS_FAILED(rv)) {
+ PrefCallback* raw = pCallback.get();
+ mObservers.InsertOrUpdate(raw, std::move(pCallback));
+ }
}
return rv;
@@ -5940,6 +5967,7 @@ nsresult PreferencesImpl::RegisterCallbackImpl(PrefChangedFunc aCallback,
bool aIsPrefix) {
MOZ_ASSERT(NS_IsMainThread());
NS_ENSURE_ARG(aCallback);
+ NS_ENSURE_FALSE(DomainEndsWithMultipleDots(aPrefNode), NS_ERROR_INVALID_ARG);
NS_ENSURE_TRUE(Preferences::InitStaticMembers(), NS_ERROR_NOT_AVAILABLE);
RefPtr<CallbackNode> node =
@@ -5969,6 +5997,7 @@ nsresult PreferencesImpl::UnregisterCallbackImpl(PrefChangedFunc aCallback,
bool aIsPrefix) {
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aCallback);
+ NS_ENSURE_FALSE(DomainEndsWithMultipleDots(aPrefNode), NS_ERROR_INVALID_ARG);
if (Preferences::sShutdown) {
MOZ_ASSERT(!Preferences::sPreferences);
return NS_OK;
diff --git a/modules/libpref/Preferences.h b/modules/libpref/Preferences.h
index 188bea22c42..8e199ba1e9a 100644
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -294,7 +294,9 @@ class Preferences final : public nsIPrefService,
// boundaries: the callback fires for aPref itself and for any pref that
// extends it by one or more whole '.'-delimited segments, but not for a pref
// that merely shares a leading substring (e.g. a "foo.bar" prefix does not
- // match "foo.barbaz"). A trailing '.' is optional and is normalized away.
+ // match "foo.barbaz"). A trailing '.' is optional and is normalized away;
+ // a domain ending in more than one '.' is rejected with
+ // NS_ERROR_INVALID_ARG.
template <typename T = void>
static nsresult RegisterPrefixCallback(PrefChangedFunc aCallback,
const nsACString& aPref,
diff --git a/modules/libpref/test/gtest/Basics.cpp b/modules/libpref/test/gtest/Basics.cpp
index db3341e4857..72abb9ec786 100644
--- a/modules/libpref/test/gtest/Basics.cpp
+++ b/modules/libpref/test/gtest/Basics.cpp
@@ -400,6 +400,30 @@ TEST(PrefsCallbackTrie, TrailingDotEquivalence)
EXPECT_EQ(count, 1);
}
+// A domain ending in more than one dot is rejected rather than partially
+// normalized.
+TEST(PrefsCallbackTrie, MultipleTrailingDotsRejected)
+{
+ int count = 0;
+ EXPECT_EQ(Preferences::RegisterCallback(IncrementCount,
+ "test.trie.dots.a.."_ns, &count),
+ NS_ERROR_INVALID_ARG);
+ EXPECT_EQ(Preferences::RegisterPrefixCallback(
+ IncrementCount, "test.trie.dots.a.."_ns, &count),
+ NS_ERROR_INVALID_ARG);
+ EXPECT_EQ(Preferences::UnregisterCallback(IncrementCount,
+ "test.trie.dots.a.."_ns, &count),
+ NS_ERROR_INVALID_ARG);
+
+ nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
+ ASSERT_TRUE(prefs);
+ RefPtr<TestWeakPrefObserver> observer = new TestWeakPrefObserver();
+ EXPECT_EQ(prefs->AddObserver("test.trie.dots.d.."_ns, observer, false),
+ NS_ERROR_INVALID_ARG);
+ Preferences::SetBool("test.trie.dots.d.e", true);
+ EXPECT_EQ(observer->mNotifyCount, 0);
+}
+
// ---------------------------------------------------------------------------
// Compact / MarkDead tests — verify lazy-unregister behavior.
// ---------------------------------------------------------------------------
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/modules/libpref/test/gtest/Basics.cpp b/modules/libpref/test/gtest/Basics.cpp
index db3341e4857..72abb9ec786 100644
--- a/modules/libpref/test/gtest/Basics.cpp
+++ b/modules/libpref/test/gtest/Basics.cpp
@@ -400,6 +400,30 @@ TEST(PrefsCallbackTrie, TrailingDotEquivalence)
EXPECT_EQ(count, 1);
}
+// A domain ending in more than one dot is rejected rather than partially
+// normalized.
+TEST(PrefsCallbackTrie, MultipleTrailingDotsRejected)
+{
+ int count = 0;
+ EXPECT_EQ(Preferences::RegisterCallback(IncrementCount,
+ "test.trie.dots.a.."_ns, &count),
+ NS_ERROR_INVALID_ARG);
+ EXPECT_EQ(Preferences::RegisterPrefixCallback(
+ IncrementCount, "test.trie.dots.a.."_ns, &count),
+ NS_ERROR_INVALID_ARG);
+ EXPECT_EQ(Preferences::UnregisterCallback(IncrementCount,
+ "test.trie.dots.a.."_ns, &count),
+ NS_ERROR_INVALID_ARG);
+
+ nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
+ ASSERT_TRUE(prefs);
+ RefPtr<TestWeakPrefObserver> observer = new TestWeakPrefObserver();
+ EXPECT_EQ(prefs->AddObserver("test.trie.dots.d.."_ns, observer, false),
+ NS_ERROR_INVALID_ARG);
+ Preferences::SetBool("test.trie.dots.d.e", true);
+ EXPECT_EQ(observer->mNotifyCount, 0);
+}
+
// ---------------------------------------------------------------------------
// Compact / MarkDead tests — verify lazy-unregister behavior.
// ---------------------------------------------------------------------------
Loading diff…
References
On This Page