High CVSS 6.5 firefox Race 🔧 Commit mapped

Overview

High
Severity
6.5
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionA race condition existed in nsHttpTransaction that could have been exploited to cause memory corruption, potentially leading to an exploitable condition.
ComponentNetworking
Bug ClassRace
Tracker1951554
Fix commitd3c35516e9e8 (firefox) +60/-27
CWECWE-362 (Race condition)
CVSS vectorCVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:L/A:N
CISA KEVNot listed
CreditedThe Mozilla Fuzzing Team
Disclosed2025-04-15

Background

`nsHttpConnectionInfo`
A ref-counted object describing a connection endpoint (host, port, proxy, origin attributes, etc.). It can be mutated/rebuilt as a transaction is restarted, re-targeted, or moved across connections.
`TakeResponseHead()` → `TakeResponseHeadAndConnInfo()`
Called on the main thread from channel code (nsHttpChannel, HttpTransactionChild, TRRServiceChannel) to take ownership of the parsed response head once headers are available.
`mLock` / thread split
nsHttpTransaction runs on the socket thread while channels consume results on the main thread; mLock guards cross-thread fields. GetConnInfo() returned the live mConnInfo without that discipline.
Alt-Svc processing
nsHttpChannel::ProcessResponse/ContinueProcessResponse1 reads the connection info to process Alt-Svc and to resume suspended responses.

Root Cause Analysis

Response processing fetched the transaction’s connection info on the main thread via RefPtr<nsHttpConnectionInfo> connInfo = mTransaction->GetConnInfo(); (in HttpTransactionChild::OnStartRequest and nsHttpChannel::ContinueProcessResponse1), while the socket thread could concurrently mutate mConnInfo (restart/re-target of the transaction). Reading/updating mConnInfo from the main thread without synchronization, concurrent with socket-thread writes, is a data race on a ref-counted pointer; in the window around the response head being taken, mConnInfo could change under the reader, yielding an inconsistent or torn nsHttpConnectionInfo reference and the memory corruption the advisory describes.

The patch removes the racy late read and instead snapshots the connection info atomically together with the response head. At activation, nsHttpTransaction::OnActivated() calls the new FinalizeConnInfo(), which clones mConnInfo into mFinalizedConnInfo under mLock. TakeResponseHead() is replaced by TakeResponseHeadAndConnInfo(nsHttpConnectionInfo** aOut), which — again under mLock (MutexAutoLock lock(mLock)) — hands back a RefPtr to mFinalizedConnInfo alongside the response head in one locked operation. The channel then threads this captured connInfo explicitly through ProcessResponse(aConnInfo) → ContinueProcessResponse1(aConnInfo) → ProcessAltService(aConnInfo), and the suspend path captures it in the resume lambda ([connInfo = RefPtr{aConnInfo}]). TRR callers that don’t need it pass nullptr. The result: the main thread consumes a stable, lock-published clone taken at activation rather than racing the socket thread’s live mConnInfo.

Key insight
The main thread read the transaction’s live, socket-thread-mutated mConnInfo without a lock during response processing; the fix snapshots a locked clone (mFinalizedConnInfo) at activation and returns it atomically with the response head, eliminating the data race.

Attack Path

  1. Trigger concurrency A server/attacker induces conditions (e.g. restart/re-target, HTTPS-RR, Alt-Svc, HTTP/3 fallback) that cause the socket thread to mutate mConnInfo while a response is being delivered (inferred from the fields involved; the diff shows the cross-thread take path).
  2. Response arrives Headers complete and the main thread enters OnStartRequest → old GetConnInfo().
  3. Race window The main-thread read of mConnInfo overlaps the socket-thread write, producing an inconsistent/stale nsHttpConnectionInfo reference.
  4. Corrupt use The bad reference flows into ProcessAltService/response processing.
  5. Primitive Memory corruption of a ref-counted networking object (advisory: “potentially leading to an exploitable condition”); concrete control is not shown by the diff and is (inferred).

Impact Assessment

The affected code runs in the networking stack (socket process/parent, main thread consuming socket-thread state); no special privilege is required, but the attacker must drive the timing of a connection-info mutation during response delivery (AC:H). The substantiated primitive is a data race on nsHttpConnectionInfo that the advisory says can cause memory corruption and may be exploitable; the diff proves the unsynchronized access and its removal but not a specific exploit primitive, so escalation beyond corruption is (inferred).

Changed Functions

FunctionChangeNotes
if
netwerk/protocol/http/HttpTransactionChild.cpp
modified
if
netwerk/protocol/http/HttpTransactionParent.cpp
modified
HttpTransactionShell
netwerk/protocol/http/HttpTransactionShell.h
modified
if
netwerk/protocol/http/TRRServiceChannel.cpp
modified
if
netwerk/protocol/http/nsHttpChannel.cpp
modified
if
netwerk/protocol/http/nsHttpTransaction.cpp
modified

Files Changed

  • netwerk/protocol/http/HttpTransactionChild.cpp
  • netwerk/protocol/http/HttpTransactionParent.cpp
  • netwerk/protocol/http/HttpTransactionShell.h
  • netwerk/protocol/http/TRRServiceChannel.cpp
  • netwerk/protocol/http/nsHttpChannel.cpp
  • netwerk/protocol/http/nsHttpChannel.h
  • netwerk/protocol/http/nsHttpTransaction.cpp
  • netwerk/protocol/http/nsHttpTransaction.h

Audit Directions

  • Other `mConnInfo`/`GetConnInfo` readers
    Audit remaining main-thread uses of the transaction’s live connection info to confirm they use a lock-published snapshot rather than the mutable member.
  • Cross-thread ref-counted fields
    Review other nsHttpTransaction members mutated on the socket thread and read on the main thread (timers, backup conn info, resolver) for the same missing-lock pattern.
  • Restart/retarget paths
    Examine Alt-Svc, HTTPS-RR, and HTTP/3 fallback code that rewrites mConnInfo to ensure finalization ordering (activation vs. mutation) is correct.
diff --git a/netwerk/protocol/http/HttpTransactionChild.cpp b/netwerk/protocol/http/HttpTransactionChild.cpp
index a707c752d69..2c0e8980bf7 100644
--- a/netwerk/protocol/http/HttpTransactionChild.cpp
+++ b/netwerk/protocol/http/HttpTransactionChild.cpp
@@ -416,7 +416,9 @@ HttpTransactionChild::OnStartRequest(nsIRequest* aRequest) {
     }
   }
 
-  UniquePtr<nsHttpResponseHead> head(mTransaction->TakeResponseHead());
+  RefPtr<nsHttpConnectionInfo> connInfo;
+  UniquePtr<nsHttpResponseHead> head(
+      mTransaction->TakeResponseHeadAndConnInfo(getter_AddRefs(connInfo)));
   Maybe<nsHttpResponseHead> optionalHead;
   nsTArray<uint8_t> dataForSniffer;
   if (head) {
@@ -482,7 +484,6 @@ HttpTransactionChild::OnStartRequest(nsIRequest* aRequest) {
     }
   }
 
-  RefPtr<nsHttpConnectionInfo> connInfo = mTransaction->GetConnInfo();
   HttpConnectionInfoCloneArgs infoArgs;
   nsHttpConnectionInfo::SerializeHttpConnectionInfo(connInfo, infoArgs);
 
diff --git a/netwerk/protocol/http/HttpTransactionParent.cpp b/netwerk/protocol/http/HttpTransactionParent.cpp
index 300cd42fa9b..95d79ead693 100644
--- a/netwerk/protocol/http/HttpTransactionParent.cpp
+++ b/netwerk/protocol/http/HttpTransactionParent.cpp
@@ -175,10 +175,17 @@ nsresult HttpTransactionParent::AsyncRead(nsIStreamListener* listener,
   return NS_OK;
 }
 
-UniquePtr<nsHttpResponseHead> HttpTransactionParent::TakeResponseHead() {
+UniquePtr<nsHttpResponseHead>
+HttpTransactionParent::TakeResponseHeadAndConnInfo(
+    nsHttpConnectionInfo** aOut) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mResponseHeadTaken, "TakeResponseHead called 2x");
 
+  if (aOut) {
+    RefPtr<nsHttpConnectionInfo> connInfo = mConnInfo;
+    connInfo.forget(aOut);
+  }
+
   mResponseHeadTaken = true;
   return std::move(mResponseHead);
 }
diff --git a/netwerk/protocol/http/HttpTransactionShell.h b/netwerk/protocol/http/HttpTransactionShell.h
index 495ab19a1ba..0c98e3b4f94 100644
--- a/netwerk/protocol/http/HttpTransactionShell.h
+++ b/netwerk/protocol/http/HttpTransactionShell.h
@@ -40,12 +40,8 @@ union NetAddr;
 //----------------------------------------------------------------------------
 
 // 95e5a5b7-6aa2-4011-920a-0908b52f95d4
-#define HTTPTRANSACTIONSHELL_IID                     \
-  {                                                  \
-    0x95e5a5b7, 0x6aa2, 0x4011, {                    \
-      0x92, 0x0a, 0x09, 0x08, 0xb5, 0x2f, 0x95, 0xd4 \
-    }                                                \
-  }
+#define HTTPTRANSACTIONSHELL_IID \
+  {0x95e5a5b7, 0x6aa2, 0x4011, {0x92, 0x0a, 0x09, 0x08, 0xb5, 0x2f, 0x95, 0xd4}}
 
 class HttpTransactionShell : public nsISupports {
  public:
@@ -101,7 +97,8 @@ class HttpTransactionShell : public nsISupports {
 
   // Called to take ownership of the response headers; the transaction
   // will drop any reference to the response headers after this call.
-  virtual UniquePtr<nsHttpResponseHead> TakeResponseHead() = 0;
+  virtual UniquePtr<nsHttpResponseHead> TakeResponseHeadAndConnInfo(
+      nsHttpConnectionInfo** aOut) = 0;
 
   // Called to take ownership of the trailer headers.
   // Returning null if there is no trailer.
@@ -193,7 +190,8 @@ NS_DEFINE_STATIC_IID_ACCESSOR(HttpTransactionShell, HTTPTRANSACTIONSHELL_IID)
       override;                                                                \
   virtual nsresult AsyncRead(nsIStreamListener* listener, nsIRequest** pump)   \
       override;                                                                \
-  virtual UniquePtr<nsHttpResponseHead> TakeResponseHead() override;           \
+  virtual UniquePtr<nsHttpResponseHead> TakeResponseHeadAndConnInfo(           \
+      nsHttpConnectionInfo** aOut) override;                                   \
   virtual UniquePtr<nsHttpHeaderArray> TakeResponseTrailers() override;        \
   virtual already_AddRefed<nsITransportSecurityInfo> SecurityInfo() override;  \
   virtual void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks)         \
diff --git a/netwerk/protocol/http/TRRServiceChannel.cpp b/netwerk/protocol/http/TRRServiceChannel.cpp
index 58094fa0241..07b3e76add1 100644
--- a/netwerk/protocol/http/TRRServiceChannel.cpp
+++ b/netwerk/protocol/http/TRRServiceChannel.cpp
@@ -1025,7 +1025,7 @@ TRRServiceChannel::OnStartRequest(nsIRequest* request) {
     // mTransactionPump doesn't hit OnInputStreamReady and call this until
     // all of the response headers have been acquired, so we can take
     // ownership of them from the transaction.
-    mResponseHead = mTransaction->TakeResponseHead();
+    mResponseHead = mTransaction->TakeResponseHeadAndConnInfo(nullptr);
     if (mResponseHead) {
       uint32_t httpStatus = mResponseHead->Status();
       if (mTransaction->ProxyConnectFailed()) {
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
index cd06902546d..9caf71adc11 100644
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -2536,7 +2536,7 @@ void nsHttpChannel::ProcessAltService(nsHttpConnectionInfo* aTransConnInfo) {
                                originAttributes, aTransConnInfo);
 }
 
-nsresult nsHttpChannel::ProcessResponse() {
+nsresult nsHttpChannel::ProcessResponse(nsHttpConnectionInfo* aConnInfo) {
   uint32_t httpStatus = mResponseHead->Status();
 
   LOG(("nsHttpChannel::ProcessResponse [this=%p httpStatus=%u]\n", this,
@@ -2691,12 +2691,13 @@ nsresult nsHttpChannel::ProcessResponse() {
   // notify "http-on-examine-response" observers
   gHttpHandler->OnExamineResponse(this);
 
-  return ContinueProcessResponse1();
+  return ContinueProcessResponse1(aConnInfo);
 }
 
-void nsHttpChannel::AsyncContinueProcessResponse() {
+void nsHttpChannel::AsyncContinueProcessResponse(
+    nsHttpConnectionInfo* aConnInfo) {
   nsresult rv;
-  rv = ContinueProcessResponse1();
+  rv = ContinueProcessResponse1(aConnInfo);
   if (NS_FAILED(rv)) {
     // A synchronous failure here would normally be passed as the return
     // value from OnStartRequest, which would in turn cancel the request.
@@ -2706,15 +2707,16 @@ void nsHttpChannel::AsyncContinueProcessResponse() {
   }
 }
 
-nsresult nsHttpChannel::ContinueProcessResponse1() {
+nsresult nsHttpChannel::ContinueProcessResponse1(
+    nsHttpConnectionInfo* aConnInfo) {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
   nsresult rv = NS_OK;
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to finish processing response [this=%p]\n",
          this));
-    mCallOnResume = [](nsHttpChannel* self) {
-      self->AsyncContinueProcessResponse();
+    mCallOnResume = [connInfo = RefPtr{aConnInfo}](nsHttpChannel* self) {
+      self->AsyncContinueProcessResponse(connInfo);
       return NS_OK;
     };
     return NS_OK;
@@ -2748,8 +2750,7 @@ nsresult nsHttpChannel::ContinueProcessResponse1() {
     }
 
     if ((httpStatus < 500) && (httpStatus != 421)) {
-      RefPtr<nsHttpConnectionInfo> connInfo = mTransaction->GetConnInfo();
-      ProcessAltService(connInfo);
+      ProcessAltService(aConnInfo);
     }
   }
 
@@ -8078,11 +8079,15 @@ nsHttpChannel::OnStartRequest(nsIRequest* request) {
     // mTransactionPump doesn't hit OnInputStreamReady and call this until
     // all of the response headers have been acquired, so we can take
     // ownership of them from the transaction.
-    mResponseHead = mTransaction->TakeResponseHead();
+    RefPtr<nsHttpConnectionInfo> connInfo;
+    mResponseHead =
+        mTransaction->TakeResponseHeadAndConnInfo(getter_AddRefs(connInfo));
     mSupportsHTTP3 = mTransaction->GetSupportsHTTP3();
     // the response head may be null if the transaction was cancelled.  in
     // which case we just need to call OnStartRequest/OnStopRequest.
-    if (mResponseHead) return ProcessResponse();
+    if (mResponseHead) {
+      return ProcessResponse(connInfo);
+    }
 
     NS_WARNING("No response head in OnStartRequest");
   }
diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h
index cb8b8b74064..c483e6a3f82 100644
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -331,9 +331,10 @@ class nsHttpChannel final : public HttpBaseChannel,
   [[nodiscard]] nsresult DispatchTransaction(
       HttpTransactionShell* aTransWithStickyConn);
   [[nodiscard]] nsresult CallOnStartRequest();
-  [[nodiscard]] nsresult ProcessResponse();
-  void AsyncContinueProcessResponse();
-  [[nodiscard]] nsresult ContinueProcessResponse1();
+  [[nodiscard]] nsresult ProcessResponse(nsHttpConnectionInfo* aConnInfo);
+  void AsyncContinueProcessResponse(nsHttpConnectionInfo* aConnInfo);
+  [[nodiscard]] nsresult ContinueProcessResponse1(
+      nsHttpConnectionInfo* aConnInfo);
   [[nodiscard]] nsresult ContinueProcessResponse2(nsresult);
   nsresult HandleOverrideResponse();
 
diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp
index 6a406fe253b..7c9e7141dd2 100644
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -217,6 +217,7 @@ nsresult nsHttpTransaction::Init(
   if (NS_FAILED(rv)) return rv;
 
   mConnInfo = cinfo;
+  mFinalizedConnInfo = cinfo;
   mCallbacks = callbacks;
   mConsumerTarget = target;
   mCaps = caps;
@@ -438,12 +439,18 @@ void nsHttpTransaction::SetH2WSConnRefTaken() {
   }
 }
 
-UniquePtr<nsHttpResponseHead> nsHttpTransaction::TakeResponseHead() {
+UniquePtr<nsHttpResponseHead> nsHttpTransaction::TakeResponseHeadAndConnInfo(
+    nsHttpConnectionInfo** aOut) {
   MOZ_ASSERT(!mResponseHeadTaken, "TakeResponseHead called 2x");
 
   // Lock TakeResponseHead() against main thread
   MutexAutoLock lock(mLock);
 
+  if (aOut) {
+    RefPtr<nsHttpConnectionInfo> connInfo = mFinalizedConnInfo;
+    connInfo.forget(aOut);
+  }
+
   mResponseHeadTaken = true;
 
   // Even in OnStartRequest() the headers won't be available if we were
@@ -523,6 +530,7 @@ void nsHttpTransaction::OnActivated() {
 
   mActivated = true;
   gHttpHandler->ConnMgr()->AddActiveTransaction(this);
+  FinalizeConnInfo();
 }
 
 void nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor** cb) {
@@ -1830,6 +1838,14 @@ static inline void RemoveAlternateServiceUsedHeader(
   }
 }
 
+void nsHttpTransaction::FinalizeConnInfo() {
+  RefPtr<nsHttpConnectionInfo> cloned = mConnInfo->Clone();
+  {
+    MutexAutoLock lock(mLock);
+    mFinalizedConnInfo.swap(cloned);
+  }
+}
+
 void nsHttpTransaction::SetRestartReason(TRANSACTION_RESTART_REASON aReason) {
   if (mRestartReason == TRANSACTION_RESTART_NONE) {
     mRestartReason = aReason;
diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h
index b0d69051db5..7164183585a 100644
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -268,6 +268,8 @@ class nsHttpTransaction final : public nsAHttpTransaction,
   already_AddRefed<Http2PushedStreamWrapper> TakePushedStreamById(
       uint32_t aStreamId);
 
+  void FinalizeConnInfo();
+
   // IMPORTANT: when adding new values, always add them to the end, otherwise
   // it will mess up telemetry.
   enum HTTPSSVC_CONNECTION_FAILED_REASON : uint32_t {
@@ -578,6 +580,9 @@ class nsHttpTransaction final : public nsAHttpTransaction,
   nsCOMPtr<nsITimer> mFastFallbackTimer;
   nsCOMPtr<nsITimer> mHttp3BackupTimer;
   RefPtr<nsHttpConnectionInfo> mBackupConnInfo;
+  // A clone of mConnInfo taken when this transaction is activated.
+  // Describes the server that the associated connection is connected to.
+  RefPtr<nsHttpConnectionInfo> mFinalizedConnInfo;
   RefPtr<HTTPSRecordResolver> mResolver;
   TRANSACTION_RESTART_REASON mRestartReason = TRANSACTION_RESTART_NONE;
 
Loading diff…