2016925 - PBroadcastChannel: Self-referential CheckPrincipalRunnable allows cross-origin message injection via forged PrincipalInfo

Status: RESOLVED FIXED

(Core :: DOM: postMessage, defect, P3)

Description

[Richard Belisle]

Attachment: test_broadcastchannel.py

BroadcastChannel (managed by PBackground) has a self-referential principal validation. CheckPrincipalRunnable (BackgroundParentImpl.cpp ~line 1230) only checks that the child-supplied PrincipalInfo deserializes to the same origin as the child-supplied origin string. Both values come from the same untrusted content process — a compromised process sends consistent values and the check always passes. No ValidatePrincipal, AssertProcessPrincipal, or ContentProcessHasPrincipal is called.

The XXX comment at ~line 1295 acknowledges: "The principal can be checked right here on the PBackground thread since BackgroundParentImpl now overrides the ProcessingError method and kills invalid child processes." Yet the actual check remains self-referential.

BroadcastChannelParent's constructor calls RegisterActor() immediately (line 21, dom/broadcastchannel/BroadcastChannelParent.cpp), before CheckPrincipalRunnable is dispatched async to the main thread. The actor is fully operational before any validation runs.

RecvPostMessage (line 33) has zero validation — forwards to BroadcastChannelService::PostMessage which routes by originChannelKey derived from child-supplied PrincipalInfo. A forged principal produces the same routing key as legitimate actors.

Impact: A compromised content process joins any origin's BroadcastChannel, reads session tokens / CSRF tokens / state sync messages, and injects arbitrary messages. PoC demonstrates full attack chain: intercept session tokens, inject fake "session expired", capture re-entered credentials.

Suggested fix: replace self-referential CheckPrincipalRunnable with ValidatePrincipal / ContentProcessHasPrincipal that cross-references the child process's actual site assignment.

Attached: write-up (submission_final.md) and Marionette PoC (test_broadcastchannel.py) demonstrating the full attack chain on Firefox ESR 140.7.0.

=== How discovered ===
Static source code analysis of dom/broadcastchannel/BroadcastChannelParent.cpp, ipc/glue/BackgroundParentImpl.cpp (CheckPrincipalRunnable, RecvPBroadcastChannelConstructor).

=== Attachments ===

1. test_broadcastchannel.py (5-phase Marionette PoC)

=== PoC prerequisites ===
pip install marionette_driver
Firefox ESR (tested on 140.7.0): apt install firefox-esr
Run: python3 test_broadcastchannel.py

Comment 1

[Andrew McCreight [:mccr8]]

This is something that would be nice to have, but we don't really attempt to enforce anywhere.

Comment 13

[Tom Schuster [:tschuster]]

Seems like this was fixed by bug 2039990: https://searchfox.org/firefox-main/rev/d5171e9e0f4729494423b0524f289073e0f0cb6c/ipc/glue/BackgroundParentImpl.cpp#823-825

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

Yes, this was fixed by bug 2039990 and the next step is bug 1853706 such that the storage key is just inherent from the managing actor. From discussion with :mccr8 I'm just marking bugs like this as dependent on that bug and fixed since those structural changes were not done as a security bug and duping to it I think will create edges that bugzilla does not hide? (I am open to other tracking approaches.)