2015298 - Compromised content process can ignore safe browsing warning from any site

Status: RESOLVED DUPLICATE of bug 2025609

(Toolkit :: Safe Browsing, defect)

Description

[Richard Belisle]

Attachment: submission.md

Howdy. I was auditing the IPC trust boundaries in Firefox's JS layer, specifically looking at how JSWindowActor message handlers in the parent process use data sent from content processes. I extracted the source from both omni.ja archives on a Firefox ESR 140.7.0 install (Kali Linux, kernel 6.18.3+kali+1-amd64), grepped across all 145 actor files for dangerous sinks in receiveMessage() handlers, then compared actor registrations to see which ones had remoteTypes restrictions and which didn't.

The BlockedSite actor jumped out because it handles security-critical operations by granting safe-browsing permissions and navigating with LOAD_FLAGS_BYPASS_CLASSIFIER — but doesn't have the remoteTypes restriction that other sensitive
actors like AboutLogins and AboutCertViewer have. I then traced the data flow from the IPC message through to the parent-side ignoreWarningLink() method and found that blockedInfo.uri goes straight into Services.perms.addFromPrincipal() and fixupAndLoadURIString() with zero validation.

I confirmed it was likely an oversight because Bug 2006509 (Jan 2026) added remoteTypes to several actors in the same registry file but didn't touch BlockedSite.

I built a PoC with Python 3.13 and marionette_driver 3.5.0 that demonstrates the permission grant against a live Firefox instance, then verified the same code is still present in mozilla-central / 147.0.3 via Searchfox.

To reproduce:

pip3 install marionette_driver
firefox-esr --marionette -remote-allow-system-access &
python3 poc_marionette.py

Comment 1

[Andrew McCreight [:mccr8]]

If an attacker has taken over a content process, bypassing Safe Browsing doesn't do much. It looks like the permission is EXPIRE_SESSION, so it isn't going to persist anything. You'd have to have some very specific attack scenario, like say a.foo.com is same site with b.foo.com and the former got put on the blocked site, and somehow bar.com has been compromised and then now it could navigate to b.foo.com, bypass the list, load a compromised page from there and use the same-site access to attack a.foo.com.

I agree with the general principle that putting a page with security-sensitive UI in a process with web content seems like a bad idea but I'm not sure it is too terrible in this case.

Also, adding a remoteType is not a valid fix for this issue because the actor is always loaded in a webIsolated content process.

Comment 2

[:Gijs (he/him)]

(In reply to Andrew McCreight [:mccr8] from comment #1)

I agree with the general principle that putting a page with security-sensitive UI in a process with web content seems like a bad idea but I'm not sure it is too terrible in this case.

Also, adding a remoteType is not a valid fix for this issue because the actor is always loaded in a webIsolated content process.

Agreed that a remoteType restriction here won't work.

We should instead remove the uri argument coming from the child and use the browsing context information from the parent.

It looks like we might be able to just rely on browsingContext.currentURI and maybe browsingContext.activeSessionHistoryEntry.triggeringPrincipal in the parent, to get them in the parent without relying on the child. But I would prefer if we could get a reference to the docshell's failedChannel as it exists in the child, in the parent, and QI to nsIClassifiedChannel there, getting all the information we need. AIUI it's always an HTTPS channel and so the networking and channel info lives in the parent anyway. Presumably the safe browsing bits happen there too...

Andrew, do you know (who knows) if there is already a path to doing this?

Comment 3

[Andrew McCreight [:mccr8]]

Maybe Nika knows?

One thing to watch out for is whether we can actually trust the currentURI field. A few of these things have Recv methods that just let you set them.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Unfortunately we don't currently preserve the failed channel in the parent process when we do a navigation. The connection between the HTTP channel and the actual error page which will be loaded is unfortunately currently discarded. It would be awesome if we could change that, but unfortunately I don't think it's super trivial to do right now.

It's especially annoying as, as you've noted, I believe that we do actually do the safe browsing checks in the parent process, so theoretically we know that the channel is going to fail in the parent process with the relevant information, and end up throwing it out :-/.

The nsHttpChannel is actually probably already dead in the parent process by this point. We kill the channel object during OnStopRequest in the child (https://searchfox.org/firefox-main/rev/52e25e8bf7d712501f99b8ba77718ea0edc42bd7/netwerk/protocol/http/HttpChannelChild.cpp#1061-1072 - the DOCUMENT case clears out the channel as well: https://searchfox.org/firefox-main/rev/52e25e8bf7d712501f99b8ba77718ea0edc42bd7/netwerk/protocol/http/HttpChannelParent.cpp#1032). I expect this will have already fired a while before the error page has loaded.

I think ideally in the future, we'd hold on to the channel and associate it with the WindowGlobalParent which ended up finishing the load, but we're currently discarding that information.

---

I think the easiest approach towards making that happen is probably to delay clearing the DocumentLoadListener field on the CanonicalBrowsingContext a bit more in success cases. Currently it's cleared as soon as we redirect into the real content process (https://searchfox.org/firefox-main/rev/52e25e8bf7d712501f99b8ba77718ea0edc42bd7/netwerk/ipc/DocumentLoadListener.cpp#1483-1487), but if we somehow delayed it even further until the load finishes in the content process, we could send down the load identifier as we construct the document and/or as we start the error page load, in order to associate them.

I don't expect changing that will be super trivial to do unfortunately.

Comment 5

[Daniel Veditz [:dveditz]]

It would be nice to fix this and be smarter about this, but the safebrowsing protection is "best effort" to begin with. The bad things you can do with a compromised child process are already more powerful

Comment 6

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:dimi, could you have a look please?

For more information, please visit BugBot documentation.

Comment 9

[Daniel Veditz [:dveditz]]

I think this was fixed in Firefox 150 by the changes in bug 2025609

Comment 10

[Richard Belisle]

Hi Daniel. I agree that this is fixed. Have a great day! - Richard

Comment 12

[Daniel Veditz [:dveditz]]

This was duped to a newer bug far outside our bug bounty collision window so it still needs consideration. The severity of the bug, however, does not qualify for a bounty