High firefox Race 🔧 Commit mapped

Overview

High
Severity
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
CISA KEVNot listed
CreditedThe Mozilla Fuzzing Team
Disclosed2025-04-15

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

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
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;
Loading diff…