High firefox Memory Corruption 🔧 Commit mapped

Overview

High
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionMemory safety bug present in Firefox 143 and Thunderbird 143. This bug showed evidence of memory corruption and we presume that with enough effort this could have been exploited to run arbitrary code.
ComponentToolkit
Bug ClassMemory Corruption
Tracker1986816
Fix commitcc9b8254d95b (firefox) +72/-29
CISA KEVNot listed
Disclosed2025-10-14

Changed Functions

FunctionChangeNotes
ConcurrentConnection
toolkit/components/places/ConcurrentConnection.cpp
modified
BlockShutdown
toolkit/components/places/ConcurrentConnection.cpp
modified
Complete
toolkit/components/places/ConcurrentConnection.cpp
modified
if
toolkit/components/places/ConcurrentConnection.cpp
modified

Files Changed

  • toolkit/components/places/ConcurrentConnection.cpp
  • toolkit/components/places/ConcurrentConnection.h
  • toolkit/components/places/FaviconHelpers.cpp
  • toolkit/components/places/FaviconHelpers.h
diff --git a/toolkit/components/places/ConcurrentConnection.cpp b/toolkit/components/places/ConcurrentConnection.cpp
index 796b3f12e60..0c039411530 100644
--- a/toolkit/components/places/ConcurrentConnection.cpp
+++ b/toolkit/components/places/ConcurrentConnection.cpp
@@ -8,6 +8,7 @@
 #include "Helpers.h"
 #include "SQLFunctions.h"
 
+#include "MainThreadUtils.h"
 #include "mozilla/AppShutdown.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/ClearOnShutdown.h"
@@ -96,7 +97,7 @@ NS_IMPL_ISUPPORTS(ConcurrentConnection, nsIObserver, nsISupportsWeakReference,
 ConcurrentConnection::ConcurrentConnection() {
   MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess(),
                         "Can only instantiate in the parent process");
-  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread(), "Must be on the main-thread");
+  AssertIsOnMainThread();
 
   // Check shutdown and try to add this as a blocker.
   nsCOMPtr<nsIAsyncShutdownService> asyncShutdownSvc =
@@ -134,6 +135,7 @@ ConcurrentConnection::ConcurrentConnection() {
 }
 
 Maybe<ConcurrentConnection*> ConcurrentConnection::GetInstance() {
+  AssertIsOnMainThread();
   static StaticRefPtr<ConcurrentConnection> sInstance;
   if (!sInstance &&
       !AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownTeardown)) {
@@ -171,7 +173,7 @@ ConcurrentConnection::GetState(nsIPropertyBag** _state) {
 // nsIAsyncShutdownBlocker
 NS_IMETHODIMP
 ConcurrentConnection::BlockShutdown(nsIAsyncShutdownClient* aBarrierClient) {
-  MOZ_ASSERT(NS_IsMainThread(), "Must be on the main-thread");
+  AssertIsOnMainThread();
   mShutdownBarrierClient = aBarrierClient;
   mState = AWAITING_DATABASE_CLOSED;
   mIsShuttingDown = true;
@@ -190,11 +192,17 @@ ConcurrentConnection::BlockShutdown(nsIAsyncShutdownClient* aBarrierClient) {
 // mozIStorageCompletionCallback
 NS_IMETHODIMP
 ConcurrentConnection::Complete(nsresult aRv, nsISupports* aData) {
-  MOZ_ASSERT(NS_IsMainThread(), "Must be on main-thread");
+  AssertIsOnMainThread();
 
-  // This is invoked only for connection opening.
+  // This is invoked as a consequence of connection opening, but the internal
+  // connection handle is not yet available, nor ready for consumption.
   MOZ_ASSERT(!mConn);
-  MOZ_ASSERT(!mIsConnectionReady);
+#ifdef DEBUG
+  mConnectionReadyMutex.NoteOnMainThread();
+  if (mIsConnectionReady) {
+    MOZ_CRASH("The connection should not be markes as ready yet");
+  }
+#endif
 
   // It's possible we got shutdown while the connection was being opened. We
   // don't even assign the connection, just try to close it.
@@ -297,8 +305,14 @@ NS_IMETHODIMP ConcurrentConnection::HandleCompletion(uint16_t aReason) {
 }
 
 void ConcurrentConnection::CloseConnection() {
-  mIsConnectionReady = false;
-  nsCOMPtr<mozIStorageAsyncConnection> conn = mConn.forget();
+  AssertIsOnMainThread();
+  nsCOMPtr<mozIStorageAsyncConnection> conn;
+  {
+    MutexAutoLock lock(mConnectionReadyMutex.Lock());
+    mConnectionReadyMutex.NoteExclusiveAccess();
+    mIsConnectionReady = false;
+    conn = mConn.forget();
+  }
 
   if (mAsyncStatements) {
     mAsyncStatements->FinalizeStatements();
@@ -329,6 +343,7 @@ void ConcurrentConnection::CloseConnectionComplete(nsresult rv) {
 
 void ConcurrentConnection::SetupConnection() {
   MOZ_ASSERT(mConn, "Connection must be defined at this point");
+  AssertIsOnMainThread();
 
   // Create common functions.
   nsresult rv = Database::InitFunctions(mConn);
@@ -346,11 +361,15 @@ void ConcurrentConnection::SetupConnection() {
     return;
   }
 
-  // Create the statements caches.
-  mAsyncStatements = MakeUnique<AsyncStatementCache>(mConn);
-  mHelperThreadStatements = MakeUnique<StatementCache>(mConn);
+  {
+    MutexAutoLock lock(mConnectionReadyMutex.Lock());
+    mConnectionReadyMutex.NoteExclusiveAccess();
+    // Create the statements caches.
+    mAsyncStatements = MakeUnique<AsyncStatementCache>(mConn);
+    mHelperThreadStatements = MakeUnique<StatementCache>(mConn);
+    mIsConnectionReady = true;
+  }
 
-  mIsConnectionReady = true;
   TryToConsumeQueues();
 }
 
@@ -385,7 +404,7 @@ nsresult ConcurrentConnection::AttachDatabase(const nsString& aFileName,
 NS_IMETHODIMP
 ConcurrentConnection::Observe(nsISupports* aSubject, const char* aTopic,
                               const char16_t* aData) {
-  MOZ_ASSERT(NS_IsMainThread());
+  AssertIsOnMainThread();
   if (strcmp(aTopic, TOPIC_PLACES_INIT_COMPLETE) == 0) {
     mPlacesIsInitialized = true;
     TryToOpenConnection();
@@ -395,7 +414,7 @@ ConcurrentConnection::Observe(nsISupports* aSubject, const char* aTopic,
 
 void ConcurrentConnection::Queue(const nsCString& aSQL,
                                  PendingStatementCallback* aCallback) {
-  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread(), "Must be on the main-thread");
+  AssertIsOnMainThread();
   if (mIsShuttingDown) {
     return;
   }
@@ -404,7 +423,7 @@ void ConcurrentConnection::Queue(const nsCString& aSQL,
 }
 
 void ConcurrentConnection::Queue(Runnable* aRunnable) {
-  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread(), "Must be on the main-thread");
+  AssertIsOnMainThread();
   if (mIsShuttingDown) {
     return;
   }
@@ -418,6 +437,11 @@ ConcurrentConnection::GetStatementOnHelperThread(const nsCString& aQuery) {
     MOZ_DIAGNOSTIC_CRASH("Use `GetStatement()` on the main-thread");
     return nullptr;
   }
+  MutexAutoLock lock(mConnectionReadyMutex.Lock());
+  mConnectionReadyMutex.NoteLockHeld();
+  if (!mIsConnectionReady) {
+    return nullptr;
+  }
   nsCOMPtr<mozIStorageStatement> stmt =
       mHelperThreadStatements->GetCachedStatement(aQuery);
   if (stmt) {
@@ -428,6 +452,7 @@ ConcurrentConnection::GetStatementOnHelperThread(const nsCString& aQuery) {
 
 already_AddRefed<mozIStorageAsyncStatement> ConcurrentConnection::GetStatement(
     const nsCString& aQuery) {
+  AssertIsOnMainThread();
   if (!NS_IsMainThread()) {
     MOZ_DIAGNOSTIC_CRASH(
         "Use `GetStatementOnHelperThread()` on the helper thread");
@@ -442,6 +467,8 @@ already_AddRefed<mozIStorageAsyncStatement> ConcurrentConnection::GetStatement(
 }
 
 void ConcurrentConnection::TryToConsumeQueues() {
+  AssertIsOnMainThread();
+  mConnectionReadyMutex.NoteOnMainThread();
   if (!mConn || !mIsConnectionReady) {
     return;
   }
@@ -472,10 +499,13 @@ void ConcurrentConnection::TryToConsumeQueues() {
 }
 
 void ConcurrentConnection::TryToOpenConnection() {
+  AssertIsOnMainThread();
+  mConnectionReadyMutex.NoteOnMainThread();
   // This is invoked at different times, thus it may try to re-enter.
   if (mIsShuttingDown || mIsOpening || mIsConnectionReady) {
     return;
   }
+
   mIsOpening = true;
 
   // Any error here means we'll be unable to do anything, thus we just shutdown.
@@ -508,6 +538,14 @@ void ConcurrentConnection::TryToOpenConnection() {
 
 void ConcurrentConnection::Shutdown() {
   MOZ_ASSERT(!mConn, "Connection should have been closed");
+#ifdef DEBUG
+  AssertIsOnMainThread();
+  mConnectionReadyMutex.NoteOnMainThread();
+  if (mIsConnectionReady) {
+    MOZ_CRASH("Connection should be closed");
+  }
+#endif
+
   mConn = nullptr;
   mIsOpening = false;
   mIsShuttingDown = true;
diff --git a/toolkit/components/places/ConcurrentConnection.h b/toolkit/components/places/ConcurrentConnection.h
index aca83a45d7b..3f50f7b61fd 100644
--- a/toolkit/components/places/ConcurrentConnection.h
+++ b/toolkit/components/places/ConcurrentConnection.h
@@ -5,6 +5,8 @@
 #ifndef mozilla_places_ConcurrentConnection_h_
 #define mozilla_places_ConcurrentConnection_h_
 
+#include "mozilla/EventTargetAndLockCapability.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/storage/StatementCache.h"
Loading diff…