2010310 - Cross-process navigation access-check gap: `WindowGlobalParent::RecvLoadURI/RecvInternalLoad` forwards loads without enforcing `CanAccess/CanNavigate`

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P3)

Description

[Steven Julian]

Summary

WindowGlobalParent::RecvLoadURI and RecvInternalLoad (parent process) forward cross-process navigations to a target BrowsingContext without enforcing the same access/navigation policy checks (e.g. CanAccess/CanNavigate) that exist for in-process navigation.

The parent handlers only:

• require the source and target BrowsingContext to be in the same BrowsingContextGroup, and
• block javascript: URLs for cross-process loads,

then directly call targetBC->LoadURI(...) / targetBC->InternalLoad(...).

This leaves a policy enforcement gap for cross-process loads initiated from a compromised content process.

Impact

A compromised content process can force navigation of other BrowsingContext instances in the same BrowsingContextGroup (including cross-origin targets), even when the attacker’s origin should not be able to control navigation of that target context.

This can be used for navigation-based security attacks such as UI redress / session confusion and as a cross-process primitive in exploit chains.

Affected code

Parent-side handlers

• dom/ipc/WindowGlobalParent.cpp
  • WindowGlobalParent::RecvLoadURI(...)
    • checks group equality and blocks javascript:
    • contains FIXME about “double check CanAccess()”
    • forwards: targetBC->LoadURI(aLoadState, aSetNavigating)
  • WindowGlobalParent::RecvInternalLoad(...)
    • same pattern; forwards: targetBC->InternalLoad(aLoadState)

IPC entry points

• dom/ipc/PContent.ipdl
  • async LoadURI(MaybeDiscardedBrowsingContext aContext, nsDocShellLoadState aLoadState, bool aSetNavigating) returns (bool aSuccess);
  • async InternalLoad(nsDocShellLoadState aLoadState);

Steps to reproduce

1. Put the attacker in control of a content process (renderer compromise / malicious code in content process).
2. From that content process, send PWindowGlobal::LoadURI or PWindowGlobal::InternalLoad targeting a different BrowsingContext in the same BrowsingContextGroup.
3. Set nsDocShellLoadState to a cross-origin URL.
4. Observe the target BrowsingContext navigates, because the parent handler forwards the load without enforcing CanAccess/CanNavigate.

result

The parent process forwards the request after only checking:

• same BrowsingContextGroup, and
• blocking javascript: URLs.

Suggested fix

Enforce access/navigation policy checks in the parent process before forwarding:

• Add a CanAccess/CanNavigate equivalent check in WindowGlobalParent::RecvLoadURI and RecvInternalLoad.
• Use the source BrowsingContext / WindowGlobal and the target BrowsingContext plus the destination URL from nsDocShellLoadState.
• Reject the IPC request when the policy check fails.

Comment 1

[Steven Julian]

I not sure, but this issue can be chained with bug 2008369 to create a more impactful exploit chain. While bug 2008369 allows a compromised content process to spoof a trusted principal (e.g., System Principal or bank origin) when invoking external protocols, this issue (2010310) enables that same compromised process to navigate other browsing contexts cross-origin without proper access checks.

Combined, these issues could allow navigation-based attacks under a spoofed principal context, increasing the potential for UI redress or privilege escalation.

cmiiw :D

Comment 2

[Artur Iunusov]

This is a really wide problem. If a process is compromised, we can't trust it. All we can do is add principal check to the parent process (assuming it's not compromised).
nsScriptSecurityManager contains some security methods though, that can be useful:
CheckLoadURIFlags
CheckLoadURIWithPrincipal

Comment 3

[Artur Iunusov]

Attachment: Bug 2010310 - Add CanNavigate() check in RecvLoadURI() and RecvInternalLoad(), r=smaug,tschuster

Comment 5

[Daniel Veditz [:dveditz]]

We don't need this hidden if we've been advertising the issue publicly in a source code comment

Comment 6

[Artur Iunusov]

implemented CanNavigate()
This is actually almost the copy of the same method from the child process

Comment 8

[Artur Iunusov]

patch is basically done
only 22 failures on treeherder
I suspect about:blank case (according to my analysis)

Comment 9

[Artur Iunusov]

basically, the following corner cases:

• about:blank
• file:// --> http:// (popups)

Comment 10

[Artur Iunusov]

all tests passed: https://phabricator.services.mozilla.com/D280216

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cb3eea4a9769
https://hg.mozilla.org/integration/autoland/rev/f822428d2b72
Add CanNavigate() check in RecvLoadURI() and RecvInternalLoad(), r=smaug,tschuster

Comment 12

[Serban Stanca [:SerbanS]]

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