CVE-2025-3608
Overview
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
nsHttpTransactionruns on the socket thread while channels consume results on the main thread;mLockguards cross-thread fields.GetConnInfo()returned the livemConnInfowithout that discipline.- Alt-Svc processing
nsHttpChannel::ProcessResponse/ContinueProcessResponse1reads the connection info to processAlt-Svcand 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.
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
- 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
mConnInfowhile a response is being delivered (inferred from the fields involved; the diff shows the cross-thread take path). - Response arrives
Headers complete and the main thread enters
OnStartRequest→ oldGetConnInfo(). - Race window
The main-thread read of
mConnInfooverlaps the socket-thread write, producing an inconsistent/stalensHttpConnectionInforeference. - Corrupt use
The bad reference flows into
ProcessAltService/response processing. - 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
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
| Function | Change | Notes |
|---|---|---|
ifnetwerk/protocol/http/HttpTransactionChild.cpp |
modified | |
ifnetwerk/protocol/http/HttpTransactionParent.cpp |
modified | |
HttpTransactionShellnetwerk/protocol/http/HttpTransactionShell.h |
modified | |
ifnetwerk/protocol/http/TRRServiceChannel.cpp |
modified | |
ifnetwerk/protocol/http/nsHttpChannel.cpp |
modified | |
ifnetwerk/protocol/http/nsHttpTransaction.cpp |
modified |
Files Changed
netwerk/protocol/http/HttpTransactionChild.cppnetwerk/protocol/http/HttpTransactionParent.cppnetwerk/protocol/http/HttpTransactionShell.hnetwerk/protocol/http/TRRServiceChannel.cppnetwerk/protocol/http/nsHttpChannel.cppnetwerk/protocol/http/nsHttpChannel.hnetwerk/protocol/http/nsHttpTransaction.cppnetwerk/protocol/http/nsHttpTransaction.h
Audit Directions
- Other `mConnInfo`/`GetConnInfo` readersAudit 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 fieldsReview other
nsHttpTransactionmembers mutated on the socket thread and read on the main thread (timers, backup conn info, resolver) for the same missing-lock pattern. - Restart/retarget pathsExamine Alt-Svc, HTTPS-RR, and HTTP/3 fallback code that rewrites
mConnInfoto ensure finalization ordering (activation vs. mutation) is correct.
Patch
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;