CVE-2025-1937
Overview
Background
- `mObserver`
- A
RefPtr<FetchDriverObserver>member. It is both read (to deliverOnDataAvailable) and cleared/consumed (on abort) — the contested shared state. - `OnDataAvailable` (OMT)
- The
nsIStreamListenerbody callback. Its own comment states “This can be called on any thread” — i.e. off-main-thread (OMT), betweenOnStartRequestandOnStopRequest. - `FetchDriverAbortActions`
- Runs on the main thread (asserts
NS_IsMainThread()); it fires the observer’sOnResponseEnd(eAborted)and then nullsmObserver. - `Mutex` / `MutexAutoLock`
- Gecko’s mutual-exclusion primitive, added here as
mODAMutexto serialize access tomObserver.
Root Cause Analysis
mObserver was accessed concurrently from two threads without synchronization. On the main thread, FetchDriver::FetchDriverAbortActions did if (mObserver) { mObserver->OnResponseEnd(eAborted, ...); mObserver = nullptr; }. Meanwhile OnDataAvailable, which the code’s own comment admits “can be called on any thread,” read mObserver into a local RefPtr (RefPtr<FetchDriverObserver> observer = mObserver;) to keep it alive while dispatching OnDataAvailable(). The original comment claimed no races were possible because OnDataAvailable is sequenced between OnStartRequest and OnStopRequest; the patch’s replacement comment corrects this, noting an abort can arrive from FetchDriverAbortActions concurrently. The unsynchronized read of a RefPtr on one thread racing with its assignment to nullptr (and the corresponding refcount drop) on another is a classic data race: the OMT thread can observe a torn/stale pointer or race the destruction of the observer it just copied, i.e. a use-after-free on the FetchDriverObserver.
The fix introduces Mutex mODAMutex (initialized in the constructor initializer list) and serializes every mObserver access that can race. In OnDataAvailable, the observer is copied into a local RefPtr under MutexAutoLock lock(mODAMutex) before being used. In FetchDriverAbortActions, mObserver is std::moved into a local under the same lock (atomically clearing the member and taking ownership), and the subsequent OnResponseEnd is called on the local copy. This guarantees that the member is never read on one thread while being cleared/released on another, closing the race (the header comment cites Bug 1810805). The advisory’s generic “memory safety bugs … evidence of memory corruption” wording is substantiated by the diff as a specific data race on a strong-reference member, not an OOB write.
RefPtr observer was read off-main-thread in OnDataAvailable while the main thread could simultaneously clear and release it in FetchDriverAbortActions, an unsynchronized data race that could drop the last reference out from under the OMT user — a use-after-free.Attack Path
- Script
Page/worker JavaScript issues a
fetch()whose response body streams in, soOnDataAvailableis invoked off the main thread. - Abort
The same script aborts the fetch (e.g. via
AbortController) at the moment body data is arriving, drivingFetchDriverAbortActionson the main thread. - Race window
Main thread enters
FetchDriverAbortActionsand reaches themObserverread/clear while the OMT thread is insideOnDataAvailablereadingmObserver(inferred timing; the diff proves the concurrent access, not the attacker’s precise scheduling). - Dangling reference
The main thread nulls
mObserverand releases the observer; the OMT thread observes a stale/racing pointer and dereferences a freedFetchDriverObserver. - Primitive Use-after-free of the observer object, which the advisory assesses as potentially exploitable for arbitrary code execution (that escalation is inferred, not shown by the diff).
Impact Assessment
AC:H, UI:R) reflects the tight thread-timing window required to win the race. The primitive is a use-after-free on a heap FetchDriverObserver; whether it yields controlled memory corruption and RCE is the advisory’s presumption, marked (inferred) here since the diff only demonstrates the race, not an exploit. Successful exploitation would be renderer-process memory corruption.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifdom/fetch/FetchDriver.cpp |
modified |
Files Changed
dom/fetch/FetchDriver.cppdom/fetch/FetchDriver.h
Audit Directions
- Other `FetchDriver` members touched OMTAudit any field read in
OnDataAvailable/OnStartRequest/OnStopRequestagainst main-thread mutators;mODAMutexonly coversmObserver. - `nsIStreamListener` implementersReview other listeners whose callbacks can fire OMT but clear strong refs on the main thread on abort/cancel (service worker, image, media loaders) for the same pattern.
- Stale "no races possible" commentsSearch for comments asserting serialization via
OnStartRequest/OnStopRequest; such reasoning ignores concurrent abort paths and may hide similarRefPtrraces.
Patch
diff --git a/dom/fetch/FetchDriver.cpp b/dom/fetch/FetchDriver.cpp
index b1c8146e753..10c6454277c 100644
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -345,6 +345,7 @@ FetchDriver::FetchDriver(SafeRefPtr<InternalRequest> aRequest,
: mPrincipal(aPrincipal),
mLoadGroup(aLoadGroup),
mRequest(std::move(aRequest)),
+ mODAMutex("FetchDriver::mODAMutex"),
mMainThreadEventTarget(aMainThreadEventTarget),
mCookieJarSettings(aCookieJarSettings),
mPerformanceStorage(aPerformanceStorage),
@@ -1458,7 +1459,10 @@ FetchDriver::OnDataAvailable(nsIRequest* aRequest, nsIInputStream* aInputStream,
uint64_t aOffset, uint32_t aCount) {
// NB: This can be called on any thread! But we're guaranteed that it is
// called between OnStartRequest and OnStopRequest, so we don't need to worry
- // about races.
+ // about races for the members accessed in OnStartRequest, OnStopRequest,
+ // FailWithNetworkError and member functions accessed before opening the
+ // channel. However, we have a possibility of a race from
+ // FetchDriverAbortActions
if (!mPipeOutputStream) {
// We ignore the body for HEAD/CONNECT requests.
@@ -1472,9 +1476,13 @@ FetchDriver::OnDataAvailable(nsIRequest* aRequest, nsIInputStream* aInputStream,
if (mNeedToObserveOnDataAvailable) {
mNeedToObserveOnDataAvailable = false;
- if (mObserver) {
+ RefPtr<FetchDriverObserver> observer;
+ {
+ MutexAutoLock lock(mODAMutex);
// Need to keep mObserver alive.
- RefPtr<FetchDriverObserver> observer = mObserver;
+ observer = mObserver;
+ }
+ if (observer) {
if (NS_IsMainThread()) {
observer->OnDataAvailable();
} else {
@@ -1871,8 +1879,13 @@ void FetchDriver::RunAbortAlgorithm() { FetchDriverAbortActions(Signal()); }
void FetchDriver::FetchDriverAbortActions(AbortSignalImpl* aSignalImpl) {
MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
+ RefPtr<FetchDriverObserver> observer;
+ {
+ MutexAutoLock lock(mODAMutex);
+ observer = std::move(mObserver);
+ }
- if (mObserver) {
+ if (observer) {
#ifdef DEBUG
mResponseAvailableCalled = true;
#endif
@@ -1880,8 +1893,7 @@ void FetchDriver::FetchDriverAbortActions(AbortSignalImpl* aSignalImpl) {
if (aSignalImpl) {
reason.set(aSignalImpl->RawReason());
}
- mObserver->OnResponseEnd(FetchDriverObserver::eAborted, reason);
- mObserver = nullptr;
+ observer->OnResponseEnd(FetchDriverObserver::eAborted, reason);
}
if (mChannel) {
diff --git a/dom/fetch/FetchDriver.h b/dom/fetch/FetchDriver.h
index c620e1452c5..349da632b0c 100644
--- a/dom/fetch/FetchDriver.h
+++ b/dom/fetch/FetchDriver.h
@@ -18,6 +18,7 @@
#include "mozilla/dom/SerializedStackHolder.h"
#include "mozilla/dom/SRIMetadata.h"
#include "mozilla/RefPtr.h"
+#include "mozilla/Mutex.h"
#include "mozilla/UniquePtr.h"
#include "mozilla/DebugOnly.h"
@@ -159,6 +160,13 @@ class FetchDriver final : public nsIChannelEventSink,
SafeRefPtr<InternalRequest> mRequest;
SafeRefPtr<InternalResponse> mResponse;
nsCOMPtr<nsIOutputStream> mPipeOutputStream;
+
+ // mutex to prevent race between OnDataAvailable (OMT) and main thread
+ // functions
+ Mutex mODAMutex;
+ // access to mObserver can race between FetchDriverAbortActions (main thread)
+ // and OnDataAvailable (OMT)
+ // See Bug 1810805
RefPtr<FetchDriverObserver> mObserver;
RefPtr<Document> mDocument;
nsCOMPtr<nsICSPEventListener> mCSPEventListener;