2023302 - Parent Crash [@ nsHttpChannel::CancelInternal] via async ORB callback

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Christian Holler (:decoder)]

Attachment: test.html

Null dereference in nsHttpChannel::CancelInternal via async ORB callback

Summary

A remote server can crash the Firefox parent process with a null-pointer
dereference by serving a cross-origin <script> response that combines
Transfer-Encoding: chunked with a lying Content-Length header, triggering
nsHttpChannel's concurrent-cache-write-interrupted recovery path which
std::move()s mResponseHead to null and falls through without restoring
it — after which the Opaque Response Blocker's async JavaScript-validation
callback fires, sets mChannelBlockedByOpaqueResponse, and calls Cancel(),
reaching an unguarded mResponseHead->ClearHeaders().

File	netwerk/protocol/http/nsHttpChannel.cpp
Crash site	Line 7445 (mResponseHead->ClearHeaders())
Root cause	Line 10557 (std::move(mResponseHead)) + line 10581 fallthrough
Bug class	Null pointer dereference (parent process)
ASan signature	SEGV on unknown address 0x000000000408
Reachable from	Web content (cross-origin <script src>)
Reproducer	brain/tests/nsHttpChannel/test.html + server.py
Default prefs	All required prefs default-on for Desktop

The bug

Crash site — nsHttpChannel.cpp:7444-7446

// CancelInternal()
if (mChannelBlockedByOpaqueResponse && mCachedOpaqueResponseBlockingPref) {
  mResponseHead->ClearHeaders();   // <<< null dereference
}

There is a sibling at nsHttpChannel.cpp:10696-10698 with the same unguarded
pattern inside ContinueOnStopRequest.

Root cause — nsHttpChannel.cpp:10543-10586

if (mCacheEntry && mCachePump && LoadConcurrentCacheAccess() &&
    aContentComplete) {
  int64_t size, contentLength;
  nsresult rv = CheckPartial(mCacheEntry, &size, &contentLength);
  if (NS_SUCCEEDED(rv)) {
    if (size == int64_t(-1)) {
      ...
    } else if (contentLength != int64_t(-1) && contentLength != size) {
      LOG(("  concurrent cache entry write has been interrupted"));
      mCachedResponseHead = std::move(mResponseHead);   // <<< mResponseHead = nullptr
      rv = MaybeSetupByteRangeRequest(size, contentLength, true);
      if (NS_SUCCEEDED(rv) && LoadIsPartialRequest()) {
        ...
        return NS_OK;   // happy path: range request
      } else {
        LOG(("  but range request setup failed ..."));
        // <<< FALLTHROUGH: mResponseHead stays nullptr, mCanceled stays false
      }
    }
  }
}

MaybeSetupByteRangeRequest(size, contentLength, true) fails when
IsResumable(size, contentLength, true) is false. IsResumable checks
(partialLen < contentLength) first — so when size >= contentLength, it
immediately returns false and we hit the fallthrough.

The code assumes "interrupted write" means size < contentLength. But
Transfer-Encoding: chunked + Content-Length: N delivers an arbitrary
body size while nsHttpResponseHead::mContentLength stays at N.

The chain

1. Content-Length / Transfer-Encoding divergence

nsHttpTransaction.cpp:2504-2516:

if (mResponseHead->Version() >= HttpVersion::v1_0 &&
    mResponseHead->HasHeaderValue(nsHttp::Transfer_Encoding, "chunked")) {
  mChunkedDecoder = new nsHttpChunkedDecoder();
  if (mContentLength != int64_t(-1)) {
    mContentLength = -1;   // transaction-local only
    ...
  }
}

The transaction's mContentLength is reset to -1 (chunked wins for body
delimiting), but nsHttpResponseHead::mContentLength is never touched —
the parsed Content-Length: 100 survives.

nsHttpHeaderArray::Flatten with pruneTransients=true (called from
InitCacheEntry → AddCacheEntryHeaders → SetMetaDataElement("response-head"))
strips Transfer-Encoding but keeps Content-Length. Result: cache metadata
says Content-Length: 100, cache data holds 200 bytes (whatever chunked
delivered).

2. Concurrent cache access

nsHttpChannel.cpp:5339-5367 (OnCacheEntryCheck):

if (size == int64_t(-1)) {   // write in progress
  ...
  if (!IsResumable(size, contentLength, true)) {
    wantCompleteEntry = true;
  } else {
    StoreConcurrentCacheAccess(1);   // <<< reader proceeds while write continues
  }
}

IsResumable(-1, 100, true): (-1 < 100) true, ignoreMissingPartialLen
handles -1 > 0, cached head has strong ETag, Accept-Ranges: bytes, no
Content-Encoding, no no-store → true. Reader sets
ConcurrentCacheAccess=1 and reads from the cache entry while the writer is
still writing.

3. Reader's ContinueOnStopRequest

Writer finishes: cache has size=200, metadata Content-Length=100.
Reader's cache pump delivers all 200 bytes through OpaqueResponseBlocker
(state Sniffing — buffers and forwards to child-process JS validator).
Cache pump fires OnStopRequest(NS_OK) → contentComplete = true.

Line 10543: all four conditions true → enter.
CheckPartial: size=200, contentLength=100.
Line 10555: 100 != -1 && 100 != 200 → enter.
Line 10557: mResponseHead becomes nullptr.
Line 10560: MaybeSetupByteRangeRequest(200, 100, true) → IsResumable
checks (200 < 100) → false → NS_ERROR_NOT_RESUMABLE.
Line 10581: fallthrough. mResponseHead == nullptr, mCanceled == false.

4. ORB OnStopRequest parks the callback

Line 10682: mListener->OnStopRequest(this, aStatus) →
OpaqueResponseBlocker::OnStopRequest. State is still Sniffing (JS
validation pending), so it just dispatches mJSValidator->OnStopRequest()
(which triggers the child-process JS parse) and returns early
(OpaqueResponseUtils.cpp:362-371).

Line 10697: mChannelBlockedByOpaqueResponse is still false (JS
validation hasn't returned yet) → skip.

ReleaseListeners(), function returns. Channel state:
mResponseHead=nullptr, mCanceled=false. Channel is kept alive by
nsCOMPtr<channel> captured in the JS-validator callback lambda
(OpaqueResponseUtils.cpp:543).

5. Async JS-validation callback fires

Event loop spins. Child process finishes JS parse — body starts with \x00,
not JavaScript. IPC MozPromise resolves → callback lambda fires
(OpaqueResponseUtils.cpp:543-582):

EnsureOpaqueResponseIsAllowedAfterJavaScriptValidation(channel, false) →
BlockOrFilterOpaqueResponse → <script> is not TYPE_FETCH so
ShouldFilterOpaqueResponse returns false → OpaqueResponse::Block →
BlockResponse(channel, NS_ERROR_FAILURE) (OpaqueResponseUtils.cpp:597-606):

void OpaqueResponseBlocker::BlockResponse(HttpBaseChannel* aChannel,
                                          nsresult aStatus) {
  mState = State::Blocked;
  mStatus = aStatus;
  aChannel->SetChannelBlockedByOpaqueResponse();   // flag = true
  aChannel->CancelWithReason(mStatus, ...);        // -> Cancel()
}

Cancel() (nsHttpChannel.cpp:7315): mCanceled is false → proceed to
CancelInternal().

CancelInternal() (nsHttpChannel.cpp:7444):
mChannelBlockedByOpaqueResponse && mCachedOpaqueResponseBlockingPref both
true → nullptr->ClearHeaders() → SEGV.

ASan trace

==55198==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000408
==55198==The signal is caused by a READ memory access.
==55198==Hint: address points to the zero page.
    #0 __pthread_mutex_lock
    #1 mozilla::RecursiveMutex::LockInternal()  RecursiveMutex.cpp:71
    #2 Lock                                     RecursiveMutex.h:31
    #3 RecursiveMutexAutoLock                   RecursiveMutex.h:80
    #4 nsHttpResponseHead::ClearHeaders()       nsHttpResponseHead.cpp:240
    #5 nsHttpChannel::CancelInternal(nsresult)  nsHttpChannel.cpp:7445
    #6 nsHttpChannel::Cancel(nsresult)          nsHttpChannel.cpp:7326
    #7 non-virtual thunk to ... Cancel
    #8 BlockResponse                            OpaqueResponseUtils.cpp:604
    #9 operator()                               OpaqueResponseUtils.cpp:562
    ...
    #14 operator()                              JSValidatorParent.cpp:41
    #15 InvokeMethod                            MozPromise.h:669
    ...
    #18 MozPromise<...>::ThenValueBase::ResolveOrRejectRunnable::Run()
    #19 SimpleTaskQueue::DrainTasks()
    #20 nsThread::DrainDirectTasks()
    #21 nsThread::ProcessNextEvent()

Address 0x408 is offsetof(nsHttpResponseHead, mRecursiveMutex) within
a null UniquePtr. Parent-process crash (crashed_parent: true).

Reproduction

# terminal 1
python3 brain/tests/nsHttpChannel/server.py 8899

# terminal 2 — prefs are all default-on for desktop; the test harness
# disables them so we re-enable:
#   browser.cache.disk.enable = true
#   browser.cache.memory.enable = true
#   network.http.rcwn.enabled = false  (optional, makes the race more reliable)
#
# load brain/tests/nsHttpChannel/test.html

The race typically wins within 1-3 rounds. The reader channel must hit
OnCacheEntryCheck during the writer's ~1.2s body-streaming window
(8 chunks × 150ms).

Server requirements

Header	Value	Why
Content-Length	100	Sets nsHttpResponseHead::mContentLength
Transfer-Encoding	chunked	Body framing ignores CL; stripped from cache
(body)	200 bytes, \x00 prefix	size > CL; fails JS parse
Content-Type	text/plain	Ambiguous → ORB Sniff → JS validation
ETag	"v1" (strong)	IsResumable() needs strong validator
Accept-Ranges	bytes	IsResumable() needs this
Cache-Control	max-age=3600	Cacheable; no no-store
(no Content-Encoding)		IsResumable() rejects encoded content

Severity / reachability

Web content	Reachable — two <script src> to an attacker server
Compromised content process	N/A — parent-process crash from content
Extensions	Same as web content

This is a remotely triggerable parent-process crash from unprivileged web
content. It requires winning a timing race (concurrent cache access), but the
server controls body pacing and the page controls request spacing, so the race
is reliable in practice.

Preconditions (all default-true on Desktop):

• browser.opaqueResponseBlocking = true (default true on Desktop, false on Android)
• browser.opaqueResponseBlocking.javascriptValidator = true (default true)
• Disk or memory cache enabled (default)

Impact is a null dereference → deterministic crash. No memory disclosure or
corruption — ClearHeaders() immediately locks mRecursiveMutex at offset
0x408 from null, which is in the zero page.

Fix

Two options:

Spot fix — add null guards

// nsHttpChannel.cpp:7444 and :10696
if (mChannelBlockedByOpaqueResponse && mCachedOpaqueResponseBlockingPref &&
    mResponseHead) {
  mResponseHead->ClearHeaders();
}

Root-cause fix — restore the move on fallthrough

// nsHttpChannel.cpp:10581
} else {
  LOG(("  but range request setup failed rv=0x%08" PRIx32
       ", failing load", static_cast<uint32_t>(rv)));
  mResponseHead = std::move(mCachedResponseHead);   // <<< restore
  aStatus = NS_ERROR_NET_INTERRUPT;
}

And likewise on the ContinueConnect() failure path at line 10578-10580.

The root-cause fix is preferable: the std::move at line 10557 is done in
anticipation of ProcessPartialContent later using mCachedResponseHead,
but when we don't take that path, mResponseHead should be put back. This
also protects against any other late caller of Cancel() (observers,
extensions, etc.).

Related

nsHttpChannel.cpp:10697 has the identical unguarded dereference in the
synchronous ContinueOnStopRequest path. It's harder to reach
(mChannelBlockedByOpaqueResponse must be true before line 10557 and
the cache pump must deliver success status after mCachePump->Cancel())
but should be fixed alongside.

nsHttpChannel.cpp:6178 (CloseCacheEntry) also dereferences
mResponseHead unguarded, but LoadInitedCacheEntry() implies
!LoadConcurrentCacheAccess() (InitCacheEntry clears it at line 6316),
so it's currently unreachable from this path. Worth a defensive guard.

Comment 1

[Christian Holler (:decoder)]

Attachment: server.py

Comment 2

[Christian Holler (:decoder)]

Attachment: fix.patch

This is the proposed fix from Claude. It would be good to evaluate if this is accurate and if it isn't, adding an explanation why.

Comment 3

[Valentin Gosu [:valentin]]

Attachment: (secure)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f52c2f07157a
https://hg.mozilla.org/integration/autoland/rev/7cb8e450943e
null check mResponseHead when calling ClearHeaders r=necko-reviewers,jesup

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7cb8e450943e