2033181 - RecvRedirect2Verify skips principal check on LoadInfo

Status: RESOLVED FIXED

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

Description

[Yaqoub Aldurayhim]

Attachment: TestMergeChildLoadInfoForwarder.cpp

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/147.0.0.0 Safari/537.36

Steps to reproduce:

1. Checked out mozilla-central at commit a2cd26231c (Nightly, 2026-04-17)
   and configured a build with ac_add_options --enable-tests in
   mozconfig.

2. Reviewed HttpChannelParent::RecvRedirect2Verify at
   netwerk/protocol/http/HttpChannelParent.cpp:849-920 and its downstream
   call to MergeChildLoadInfoForwarder at
   ipc/glue/BackgroundUtils.cpp:1131-1184. No principal validation on
   ChildLoadInfoForwarderArgs.reservedClientInfo.principalInfo,
   .initialClientInfo.principalInfo, or .controller.principalInfo
   before they are applied to nsILoadInfo via
   OverrideReservedClientInfoInParent / SetInitialClientInfo /
   SetController.

3. Wrote ipc/glue/test/gtest/TestMergeChildLoadInfoForwarder.cpp
   (attached) — four tests that:

   • Create an HTTP channel on http://attacker.example.
   • Build ChildLoadInfoForwarderArgs with a bank.example
ContentPrincipalInfo inside reservedClientInfo and a
     bank.example IPCServiceWorkerDescriptor inside controller.
   • Call MergeChildLoadInfoForwarder(forged, channel->LoadInfo()).
   • Assert that the attacker-channel's LoadInfo now reports bank.example
     as its reserved-ClientInfo origin and controller scope, and that an
     empty forwarder unconditionally clears a pre-seeded controller.

4. Registered the test in sibling moz.build (attached as
   moz.build.patch).

5. Ran:

    ./mach build
    ./mach gtest 'MergeChildLoadInfoForwarder*'
   

   All four tests pass on the above Nightly revision (output log attached,
   see "Test results" under "What happened?").

IPDL message the attacker primitive corresponds to (what a compromised
content process would place on the wire to reach the same gap through the
real PHttpChannel actor — the GTest reaches the gap directly because
MergeChildLoadInfoForwarder is a free function):

[PHttpChannel] Redirect2Verify(
    result              = NS_OK,
    changedHeaders      = {},
    sourceRequestBlockingReason = 0,
    targetLoadInfoForwarder = Some(ChildLoadInfoForwarderArgs{
        reservedClientInfo = Some(IPCClientInfo{
            principalInfo = ContentPrincipalInfo{
                originNoSuffix = "https://bank.example",
                spec           = "https://bank.example/",
                attrs          = default,
            },
            id            = fresh-UUID,
            url           = "https://bank.example/app/x",
            type          = Window,
            frameType     = None,
        }),
        initialClientInfo    = Nothing,
        controller = Some(IPCServiceWorkerDescriptor{
            principalInfo   = <same bank.example ContentPrincipalInfo>,
            scope           = "https://bank.example/app",
            scriptURL       = "https://bank.example/sw.js",
            state           = Activated,
            handlesFetch    = true,
        }),
        requestBlockingReason = 0,
    }),
    loadFlags           = LOAD_REPLACE,
    referrerInfo        = null,
    apiRedirectURI      = null,
    corsPreflightArgs   = Nothing)

A controller = Nothing variant additionally demonstrates that the
aLoadInfo->ClearController() call at BackgroundUtils.cpp:1170 is
unconditional — any forwarder with controller = Nothing clears a
pre-existing SW controller on the redirected channel, no cooperating SW
required.

Note on attacker model: this IPC message has no JS/WebIDL binding, so
only an attacker who has already achieved code execution inside a content
process (e.g. via a memory-safety bug) can send it. The parent is expected
to validate principals on such IPCs — that's the gap here.

Actual results:

HttpChannelParent::RecvRedirect2Verify unconditionally calls
MergeChildLoadInfoForwarder(aTargetLoadInfoForwarder.ref(), newLoadInfo)
when aTargetLoadInfoForwarder.isSome():

• netwerk/protocol/http/HttpChannelParent.cpp:849-855 (handler signature)
• netwerk/protocol/http/HttpChannelParent.cpp:913-920 (unconditional merge call)

MergeChildLoadInfoForwarder then:

• ipc/glue/BackgroundUtils.cpp:1160-1165 calls
  aLoadInfo->OverrideReservedClientInfoInParent(reservedClientInfo.ref())
• ipc/glue/BackgroundUtils.cpp:1166-1168 calls
  aLoadInfo->SetInitialClientInfo(initialClientInfo.ref())
• ipc/glue/BackgroundUtils.cpp:1170 calls
  aLoadInfo->ClearController() (unconditional)
• ipc/glue/BackgroundUtils.cpp:1171-1174 calls
  aLoadInfo->SetController(ServiceWorkerDescriptor(controller.ref()))
• ipc/glue/BackgroundUtils.cpp:1176-1181 calls
  aLoadInfo->SetRequestBlockingReason(blockingReason)

No caller on this path validates:

• reservedClientInfo.PrincipalInfo() against
  newHttpChannel->LoadInfo()->TriggeringPrincipal() / GetURI().
• controller.principalInfo against the channel URL or SW scope registry.
• ServiceWorkerManager::GetActiveWorkerInfoForScope that the claimed SW
  registration actually exists for that principal.

Exhaustive grep of netwerk/protocol/http/HttpChannelParent.cpp (current
trunk) for ValidatePrincipal, ValidatePrincipalCouldPotentiallyBeLoadedBy,
or CheckBrowsingContextEmbedder on the Redirect2Verify path: zero
matches.

Observable post-conditions on the parent (demonstrated by the attached
GTest):

newHttpChannel->LoadInfo()->GetReservedClientInfo()->PrincipalInfo()
    → reports attacker-chosen origin

newHttpChannel->LoadInfo()->GetController()
    → reports attacker-chosen ServiceWorkerDescriptor
      (or Nothing, in the ClearController bypass variant)

Downstream:

• FetchDriver::MainFetch / nsHttpChannel::MaybeDispatchFetchEvent reads
  loadInfo->GetController() to decide SW interception; the forged
  controller causes dispatch to the attacker-named SW at the attacker-named
  scope (if the channel URL falls in scope), or skips SW interception
  entirely in the ClearController variant — defeating SW-enforced login
  gates, CSP shims, and offline-auth SWs on the victim origin.
• ServiceWorkerManager::DispatchFetchEvent reads
  loadInfo->GetReservedClientInfo() at
  dom/serviceworkers/ServiceWorkerManager.cpp:2270 and calls
  StartControllingClient(clientInfo, registration) at line 2332. The
  principal-mismatch fix-up block at 2303-2326 is explicitly gated
  if (!XRE_IsParentProcess()), so forged principals applied via
  MergeChildLoadInfoForwarder are not reconciled on the parent-side
  interception path.
• Post-redirect ClientSource association is routed to a
  ClientManagerService entry under the forged principal.

Test results (2026-04-18)

Currently running suites: MergeChildLoadInfoForwarder
SUITE_START: gtest - running 0 tests
TEST_START: MergeChildLoadInfoForwarder.ForgedReservedClientInfo_CrossOriginPrincipal
TEST_END: PASS
TEST_START: MergeChildLoadInfoForwarder.ClearController_UnconditionalBypass
TEST_END: PASS
TEST_START: MergeChildLoadInfoForwarder.ForgedController_CrossOriginSWDescriptor
TEST_END: PASS
TEST_START: MergeChildLoadInfoForwarder.RequestBlockingReason_ChildControls
TEST_END: PASS
SUITE_END

Ran 4 checks (4 tests)
Expected results: 4
Unexpected results: 0
OK
Ran 4 test(s) from 1 test suite(s) (0 disabled), with 0 failure(s).

All four tests pass, confirming that (1) forged cross-origin
reservedClientInfo lands on the LoadInfo unvalidated, (2) an empty
forwarder unconditionally clears a pre-seeded SW controller, (3) forged SW
descriptors with a cross-origin principal land via SetController, and
(4) attacker-chosen requestBlockingReason is written verbatim.

Expected results:

Under Firefox's Fission trust model, any principal-bearing struct crossing
the content→parent IPC boundary must be validated against principals the
parent independently trusts. The established pattern is
ContentParent::ValidatePrincipal /
ValidatePrincipalCouldPotentiallyBeLoadedBy, applied to PrincipalInfo
fields on other parent-side IPC handlers (e.g. FetchParent migrated in
this pattern).

Before calling OverrideReservedClientInfoInParent, SetInitialClientInfo,
or SetController, the handler should verify that each principal-bearing
field is consistent with the channel's authoritative principals:

// In HttpChannelParent::RecvRedirect2Verify, before calling
// MergeChildLoadInfoForwarder:
if (aTargetLoadInfoForwarder.isSome()) {
  const auto& fw = aTargetLoadInfoForwarder.ref();
  RefPtr<ContentParent> cp =
      static_cast<ContentParent*>(Manager()->Manager());
  auto checkPI = [&](const mozilla::ipc::PrincipalInfo& pi) -> bool {
    auto r = PrincipalInfoToPrincipal(pi);
    if (r.isErr()) return false;
    nsCOMPtr<nsIPrincipal> p = r.unwrap();
    return cp->ValidatePrincipal(
        p, {ContentParent::ValidatePrincipalOptions::AllowSystem});
  };
  if (fw.reservedClientInfo().isSome() &&
      !checkPI(fw.reservedClientInfo().ref().principalInfo())) {
    return IPC_FAIL(this, "reservedClientInfo principal not valid for sender");
  }
  if (fw.initialClientInfo().isSome() &&
      !checkPI(fw.initialClientInfo().ref().principalInfo())) {
    return IPC_FAIL(this, "initialClientInfo principal not valid for sender");
  }
  if (fw.controller().isSome() &&
      !checkPI(fw.controller().ref().principalInfo())) {
    return IPC_FAIL(this, "SW controller principal not valid for sender");
  }
}
// then call MergeChildLoadInfoForwarder as today

Additionally, for the controller specifically, consult
ServiceWorkerManager::GetActiveWorkerInfoForScope to confirm the claimed
registration actually exists for that principal before trusting
SetController.

Comment 1

[Yaqoub Aldurayhim]

Attachment: Register TestMergeChildLoadInfoForwarder in moz.build

Comment 2

[Yaqoub Aldurayhim]

Attachment: gtest_output.log

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: (secure)

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Downstream exploitability: The report's claim about SW interception hijacking is plausible but the actual exploit chain is complex. A compromised content process would need to:

1. Already control a navigation/sub-resource load that triggers a redirect
2. Time the Redirect2Verify message with forged args
3. Have the forged SW controller match a scope that the redirect target falls under

The ClearController bypass is simpler — any redirect verification clears the existing controller. But for a cross-origin redirect, Fission would typically route the new page to a different content process, limiting the damage window to parent-side interception decisions.

requestBlockingReason forgery (test #4) is low severity — it's a uint32_t enum, not a principal, and can only set a non-zero blocking reason (the code at line 1176-1180 only overrides when non-zero).

Comment 6

[Daniel Veditz [:dveditz]]

Under Firefox's Fission trust model, any principal-bearing struct crossing
the content→parent IPC boundary must be validated against principals the
parent independently trusts

That's the desired state, but we are aware we shipped Fission before that work was completed because it was important to gain the site isolation benefits in the non-compromised-process case.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0196001b628c
https://hg.mozilla.org/integration/autoland/rev/cb3396fe1023
Check for active ServiceWorkers r=necko-reviewers,dom-storage-reviewers,dom-worker-reviewers,valentin,edenchuang

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/cb3396fe1023

Comment 9

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:jesup, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox153 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D295439

Comment 11

[Phabricator Automation]

firefox-esr153 Uplift Approval Request

• User impact if declined/Reason for urgency: Security risk
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: straightforward patch
• String changes made/needed?: none
• Is Android affected?: yes

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr153/rev/081dfb5b7074