1783504 - (CVE-2024-0748) Compromised content process can UpdateDocumentURI

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Young Min Kim]

Attachment: poc.patch

WindowGlobalParent::RecvUpdateDocumentURI does not validate the new DocumentURI, so a compromised content process is able to set an arbitrary documentURI (https://searchfox.org/mozilla-central/rev/675d6b7b04c4a802493d29632ee45f6e036a5702/dom/ipc/WindowGlobalParent.cpp#370).

This is used in several places including but not limited to:

1. Address bar and related components, when the history is updated, e.g., history.pushState() or history.back().

2. Hub.fillInAddress for extensions/ConduitsParent (https://searchfox.org/mozilla-central/rev/675d6b7b04c4a802493d29632ee45f6e036a5702/toolkit/components/extensions/ConduitsParent.jsm#187)
   This sets MessageSender.url for extension messages. Many extensions rely on MessageSender.url to verify whether the message came from a trusted source.

3. webrtcUI.getActiveStreams (https://searchfox.org/mozilla-central/rev/675d6b7b04c4a802493d29632ee45f6e036a5702/browser/modules/webrtcUI.jsm#180)
   This is used in "Tabs sharing devices" menu (the permission prompt is not affected).

4. LoginAutoComplete::startSearch (https://searchfox.org/mozilla-central/rev/675d6b7b04c4a802493d29632ee45f6e036a5702/toolkit/components/passwordmgr/LoginAutoComplete.jsm#531) and LoginManagerChild::_filterForExactFormOriginLogins (https://searchfox.org/mozilla-central/rev/675d6b7b04c4a802493d29632ee45f6e036a5702/toolkit/components/passwordmgr/LoginManagerChild.jsm#2646)
   It seems only fill via the context menu is possible, autofill seems not affected.

Threrefore, the compromised content process can spoof the address bar, extension message from trusted component, Tabs sharing devices" menu, and get logins for another origin.

REPRODUCTION CASE

1. Apply the attached poc.patch.
2. Create a new login for https://example.com in about:logins.
3. Install the attached extension.
4. Visit the attached poc.html over secure origin (an online version is available in https://chs17f87gytb304hg6vk.glitch.me/)
5. Press "Click" and check the address bar.
6. Check the extension message sender.
7. Press "getUserMedia(audio)" and check "Tabs sharing devices" menu.
8. 5-7 should show example.com.
9. Open context menu on the Username field and select the login added in 2. Check the password field is filled.

Comment 1

[Young Min Kim]

Attachment: ext.zip

Comment 2

[Young Min Kim]

Attachment: poc.html

Comment 3

[Andrew McCreight [:mccr8]]

Nika, it looks like you have a comment in this function that sounds like the basic issue described in comment 0. Do we have a bug on file for this?

Comment 4

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

I wasn't able to find one with a quick search. When I first wrote this code the WindowGlobalParent version of things wasn't used for anything yet, so there wasn't much of an impact, but we should definitely try to do better here.

It would be nice to do the full suite of checks which would be performed by a call like pushState here, however it's likely we'll need to relax things slightly at least at first while we're working on this. Perhaps we could start out by just having an origin check between the previous URI and a new URI, or some other check to make sure that the origin of the new URI matches mDocumentPrincipal. There will need to be some other checks to allow e.g. about:blank or about:srcdoc in some cases as well.

Comment 5

[Daniel Veditz [:dveditz] out until Nov 10]

It's not quite a sandbox escape yet, but there might be some front-end code that would grant additional privileges to a chrome or about URL. Otherwise, you can definitely use this for spoofing and violating same-origin, which is sec-moderate if it requires a compromised process to get there.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

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

For more information, please visit auto_nag documentation.

Comment 7

[Andrew McCreight [:mccr8]]

I think we should fix this at some point and not just let it linger. It sounds like there's a reasonable path forward here to at least get some protection.

Comment 8

[Andrew McCreight [:mccr8]]

Jens, could you maybe find somebody to work on this? I'm not who exactly is responsible for WindowGlobalParent stuff at this point. Nika laid out a plan of attack in comment 4, but doesn't have time to work on it. It is a little open ended, but it feels like a reasonably scoped project. Thanks.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

In addition to documentURI, we probably want to tweak isInitialDocument.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Taking this, although I may not be the one working on this (but rather helping someone else to fix this).

Comment 11

[Artur Iunusov]

Attachment: Bug 1783504 - tweak when UpdateDocumentURI can be set, r=smaug

Comment 12

[Artur Iunusov]

Attachment: WIP: Bug 1783504 - FIX Compromised content process can UpdateDocumentURI, r=smaug!

Differencial Revision: https://phabricator.services.mozilla.com/D159340

Comment 13

[Artur Iunusov]

Attachment: WIP: Bug 1783504 - FIX Compromised content process can UpdateDocumentURI, r=smaug!

Comment 14

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

FIX Compromised content process can UpdateDocumentURI, r=smaug,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/a364609f354a7d00bd1d44c0272b21b3eeb9b177
https://hg.mozilla.org/mozilla-central/rev/a364609f354a

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.

Comment 16

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

Backed out on request from Artur for causing frequent crashes (bug 1832321):
https://hg.mozilla.org/mozilla-central/rev/045b0358990bc474903d8c11101271faeec8482c

Comment 17

[Artur Iunusov]

Backed out. No need to nominate for beta

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:aiunusov, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 19

[Artur Iunusov]

Sure. Thanks!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29c24651d5f3
Add scheme logging to investigate a crash, r=smaug,necko-reviewers

Comment 21

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

https://hg.mozilla.org/mozilla-central/rev/29c24651d5f3

Comment 22

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/614d9a698c47
Backed out changeset 29c24651d5f3 for causing Android crashes. a=backout

Comment 23

[Artur Iunusov]

https://hg.mozilla.org/mozilla-central/rev/614d9a698c47037b40ce12a685db6bb2eba75442

Comment 24

[Young Min Kim]

Please note this issue will be published and presented in the upcoming USENIX Security on Friday, Auguest 11. Only the MessageSender.url part will be mentioned and neither PoC nor the detail will be made public.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81a597c0aa27
Move security check to the content process, r=smaug

Comment 26

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

https://hg.mozilla.org/mozilla-central/rev/81a597c0aa27

Comment 27

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

Backed out previous changeset on request from aiunusov because a sufficient amount of crash reports has been collected:
https://hg.mozilla.org/integration/autoland/rev/62357f3d85016b6d944d6da24d20f3a579efc623

Comment 28

[Jens Stutte [:jstutte]]

Interesting, in all the collected Fenix crashes we come through if (!mozilla::SessionHistoryInParent()) after a GoBack. That would explain why it does not happen elsewhere, IIUC SHIP is active everywhere except for Android?

Comment 29

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

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/62357f3d8501

Comment 30

[Olli Pettay [:smaug][[e-mail]]]

The problem is likely again in https-only mode

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/724cb235ebec
tweak when UpdateDocumentURI can be set, r=smaug

Comment 32

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

https://hg.mozilla.org/mozilla-central/rev/724cb235ebec

Is there anything left to be done here or can this bug be closed?

Comment 33

[Artur Iunusov]

We need to monitor https://crash-stats.mozilla.org for the possible crashes, at least, for one week.
If this patch will not be backed out afterwards - we can close it

Comment 34

[Jens Stutte [:jstutte]]

Trying to capture a thought we had when talking Artur and I:

The crashes we see (again) have HandleSameDocumentNavigation on their stack by chance, as there seem to be an expensive synchronous execution. In fact, SendDocumentURI is asynchronous and we cannot be 100% sure that the RecvDocumentURI that caused the dump corresponds to the SendDocumentURI inside this HandleSameDocumentNavigation call - it might just have been an earlier invocation of SendDocumentURI that might have come from different code paths. There is a time window between sending the IPC message on the child side, elaborating it on the parent side and then requesting a dump from the client that could be arbitrarily long, such that we end up scheduling a second SendDocumentURI in the meantime. It might be worth looking out for any SetDocumentURI invocation on the child side, especially if coming from Java code.

Comment 35

[Sandor Molnar[:smolnar]]

Backed out for crashes on crash stats on Android Nightly

Backout link: https://hg.mozilla.org/integration/autoland/rev/571c293cedd28db6ac86fd553e2810f40e382e80

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

SetDocumentURI coming from client side from java code? Can't figure out what that might mean.

Comment 37

[Sandor Molnar[:smolnar]]

backout merge to central: https://hg.mozilla.org/mozilla-central/rev/571c293cedd2

Comment 38

[Jens Stutte [:jstutte]]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #36)

SetDocumentURI coming from client side from java code? Can't figure out what that might mean.

That I don't know, but I just wanted to point out that we might not come through the code path we think we are coming through on the child side, at least in theory.

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16c2fd641193
tweak when UpdateDocumentURI can be set, r=smaug

Comment 40

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

https://hg.mozilla.org/mozilla-central/rev/16c2fd641193

Comment 41

[Artur Iunusov]

no crashes during this week so far: https://crash-stats.mozilla.org/signature/?signature=IPCError-content%20%7C%20RecvUpdateDocumentURI%20Setting%20DocumentURI%20with%20a%20different%20Origin%20than%20principal%20URI&date=%3E%3D2023-11-17T17%3A21%3A00.000Z&date=%3C2023-11-24T17%3A21%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#reports

I guess we can wait for another week, and then close the bug

Comment 42

[Ryan VanderMeulen [:RyanVM]]

What are your thoughts on backports here? It grafts cleanly to both Beta but needs a bit of rebasing for ESR115. But I'm also wondering how much bake time you think we need?

Comment 43

[Artur Iunusov]

We need to test this patch at least until next soft freeze
I would target a Beta at this moment

Comment 44

[Ryan VanderMeulen [:RyanVM]]

Alright, let's let this ride Fx122 then and we can revisit the ESR backport next cycle.

Comment 45

[Artur Iunusov]

Sounds good

Comment 46

[Artur Iunusov]

If we are going to merge it to esr 115, then I must to prepare this patch and https://bugzilla.mozilla.org/show_bug.cgi?id=1868387
In order to have a pref (to disable the whole feature in case of an incident)

Links to monitor crashes:
https://crash-stats.mozilla.org/search/?signature=~SetDocumentURI&date=%3E%3D2023-12-14T13%3A01%3A00.000Z&date=%3C2023-12-21T13%3A01%3A00.000Z&_facets=signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

https://crash-stats.mozilla.org/search/?signature=~RecvUpdateDocumentURI&date=%3E%3D2023-12-14T12%3A57%3A00.000Z&date=%3C2023-12-21T12%3A57%3A00.000Z&_facets=signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

(so far, amount is quite low)

Comment 47

[Artur Iunusov]

These patches requires uplift (if they not in the release already):

https://phabricator.services.mozilla.com/D159340
https://phabricator.services.mozilla.com/D196290
https://phabricator.services.mozilla.com/D195554
https://phabricator.services.mozilla.com/D197698

(sounds like I have no enough rights to create Uplift form)

Comment 48

[Artur Iunusov]

Just want to make sure, that all the patches mentioned above will go to release before soft freeze.
But I have no rights for doing uplift.

could you, please, guide me, how to proceed in that case?

Comment 49

[Ryan VanderMeulen [:RyanVM]]

All of those are on track to ride 122 to Release. If you want to nominate them for ESR approval also, you can do so by setting the approval-mozilla-esr115 dropdown to ? on the Details link for each attachment and fill out the form. That said, the urgency probably isn't very high there since bug 1873052 would disable the behavior anyway.

Comment 50

[Artur Iunusov]

all clear, then. Thanks!

Comment 51

[Daniel Bodea [:danibodea]]

(In reply to Young Min Kim from comment #0)

REPRODUCTION CASE

1. Apply the attached poc.patch.
2. Create a new login for https://example.com in about:logins.
3. Install the attached extension.
4. Visit the attached poc.html over secure origin (an online version is available in https://chs17f87gytb304hg6vk.glitch.me/)
5. Press "Click" and check the address bar.
6. Check the extension message sender.
7. Press "getUserMedia(audio)" and check "Tabs sharing devices" menu.
8. 5-7 should show example.com.
9. Open the context menu on the Username field and select the login added in 2. Check the password field is filled.

I am unable to reproduce using the REPRODUCTION CASE in comment 0. Could you help clear out the steps to reproduce so that I know how to properly reproduce and verify the fix? Questions:

1. Could a QA do the first step? How do I apply this patch? Is this not a build compilation process?
2. Regarding step 2, should I create a login for https://example.com? Should I not create logins for the test page's location https://chs17f87gytb304hg6vk.glitch.me/ instead?
3. Regarding step 3, should I add the attached extension as a temporary addon?
4. Regarding step 4, I assume I should be loading the specified link (https://chs17f87gytb304hg6vk.glitch.me/), right?
5. Regarding step 5, what should I check about the address bar?
6. Regarding step 6, what should I check, exactly?
7. Regarding step 7, should I allow the permission that is displayed after clicking "getUserMedia(audio)"? What should I be checking about the Permissions Menu?
8. Regarding step 8, What exactly should be showing "example.com"?
9. Do I left-click the Username field to select the saved login? Or do I have to use right-click and "Use saved logins"?
10. The affected behavior is that the password is not autocompleted when selecting the username, so the fixed behavior is that the password is not being auto-filled, right?

I am sorry to ask such specific questions. I am QA and do not possess the technical level of development engineers. We (QA) need to make sure we reproduce the issue to properly verify it's fixed. Thank you!

Comment 52

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt

Comment 53

[Artur Iunusov]

1 Need to patch a code. I can do this for you
2 yes, you have to go to the about:logins and create a login
3 - yes, extension is needed
4 https://chs17f87gytb304hg6vk.glitch.me - exactly that link
5 yes
6 - you should expect a tab crash
7 yes, and expect tab crash
8 tab must crash

Comment 54

[Artur Iunusov]

I will prepare a link with the patched version for the testing soon

Comment 55

[Daniel Bodea [:danibodea]]

This message serves as a reminder to address the QA requirement in order to reproduce and verify the fix. Thank you.

Comment 56

[Artur Iunusov]

Prepared builds for QA:
https://treeherder.mozilla.org/jobs?repo=try&revision=f719296de0493d23f647ff2e6372a636bfadcb28

Writing the instruction at the moment

Comment 57

[Daniel Bodea [:danibodea]]

The steps in comment 0 still don't cause a crash while using the newly created patch build. It's important to notice that the extension does not seem to work even though it loads as a temporary addon. Furthermore, I would like to know why we should consider the fix valid considering the reproduction needs a different build than the officially released ones. Should we be expecting new instructions? Thank you.

Comment 58

[Artur Iunusov]

We need a patched build in order to simulate a compromised content process
(I am checking that patched version at the moment)

Comment 59

[Artur Iunusov]

Attachment: poc.patch

Updated POC

Comment 60

[Artur Iunusov]

Figured out. It seems that poc.patch was a bit outdated (after the recent changes in Firefox itself).
I prepared a new version of the patched builds, and tested them locally:
https://treeherder.mozilla.org/jobs?repo=try&revision=49861ebd428cd9d9de3610c72d200b39cdc021fa
Tab must crash in that version.

Could you please, check again?
(we cannot use the normal nightly for the testing, since we do not know yet, how to compromise the content process in a real scenario)

Comment 61

[Artur Iunusov]

0 Download a patched version from https://treeherder.mozilla.org/jobs?repo=try&revision=49861ebd428cd9d9de3610c72d200b39cdc021fa
1 Create a new login for https://example.com in about:logins.
2 Install the attached extension.
3 Visit the attached poc.html over secure origin (an online version is available in https://chs17f87gytb304hg6vk.glitch.me/)

(after a visiting poc.html or https://chs17f87gytb304hg6vk.glitch.me/, tab must immediately crash)

Comment 62

[Daniel Bodea [:danibodea]]

I can reproduce this tab crash using the build and extension provided in the previous comment on Windows 10 and 11, but the creation of a login for https://example.com/ is not necessary. The tab crashes with and without it. Is this a correct reproduction of the issue in question?

Furthermore, how should I proceed to verify this fix considering that the reproduction was made on a patched build to simulate a compromised content process? Thank you.

Comment 63

[Artur Iunusov]

I think creating a new login isn't really necessary (since the tab will crash anyway before a compromised process tries to even access it).

Regarding the second question, I think, reading POC.patch might help (which compromises a content process in a simple and dirty way) and building a version for verification on your machine:

https://firefox-source-docs.mozilla.org/setup/windows_build.html
https://firefox-source-docs.mozilla.org/setup/linux_build.html
https://firefox-source-docs.mozilla.org/setup/macos_build.html
(Just one extra command is required in order to patch it: hg import POC.patch)

You can also play with the new setting (dom.security.setdocumenturi). For example, disable it and perform the steps (4-9).

Comment 64

[Daniel Bodea [:danibodea]]

Unfortunately (as QA) we don't have the required time and resources to patch our test builds. Would you be able to patch another build for us so we can verify it? Thank you.

Comment 65

[Artur Iunusov]

Sure. I can patch any revision you ask.
If you need just a patched version of today's Nightly, I will provide it soon today

Comment 66

[Daniel Bodea [:danibodea]]

It would be best to verify it in the latest Nightly and latest Release if it's not too much to ask. Thank you for your support!

Comment 67

[Artur Iunusov]

Builds are on their way: https://treeherder.mozilla.org/jobs?repo=try&revision=a8142806fa2ea2b1c58702bd049c015a2de947bb

Comment 68

[Daniel Bodea [:danibodea]]

This issue still occurs with the provided Nightly v125.0a1 try build using the provided steps.
Furthermore, Microsoft Defender has detected a virus in the provided try build: Trojan:Script/Wacatac.B!ml
I cannot verify the fix in either of the latest branches.

Comment 69

[Artur Iunusov]

The provided binaries contained only this change: https://hg.mozilla.org/try/rev/02babe736726c6e21e1a4783009d14b6a667b20a
which is of course, could be detected by the antivirus as malicious software. Since this POC intentionally makes a content process compromised (for testing purposes only)

Comment 70

[Artur Iunusov]

Could you please, explain the issue with Nightly v125.0a1?
In any case I can provide a new binaries for testing

How about this build (this is today's Nightly, with the patched content process, in order to be 'compromised'): https://treeherder.mozilla.org/jobs?repo=try&revision=137cf9c00e4f7b565da87be8fc05d11937ac3f75

in case of any issues, you can also ping me in Matrix :)

Comment 71

[Artur Iunusov]

UPD: I double checked everything, and managed to reproduce the tab crash using the steps above and binaries from https://treeherder.mozilla.org/jobs?repo=try&revision=137cf9c00e4f7b565da87be8fc05d11937ac3f75

Comment 72

[Artur Iunusov]

The original issue has been fixed. Now, instead of leaking data, the tab crashes immediately.
But since we don't yet know if it's even possible to compromise the content process, it makes for QA too difficult to test. This is why custom binaries are needed.

Comment 73

[Daniel Bodea [:danibodea]]

From the answers in comment 53, I have understood that to reproduce the issue, the tab must crash so I expected the tab not to crash in the fixed builds. In my testing, the same outcome was observed in both the "affected" build and the "fixed" one as the tab crashes.

As a consequence, I need to ask for more details: Assuming that the data leak is the actual issue, how am I to determine whether the build leaks data or not so I can correctly verify the fix?

Thank you for your help!

Comment 74

[Artur Iunusov]

In short, this is the correct behavior: the tab should crash, in order not to allow to change a document URI and steal the data.
Basically, we're dealing with a security issue that not exist, but could arise in the future if someone finds a way to compromise the content process (that is why testing is so unusual and sometimes complicated)

(I think, to confirm that fix is working, I could also provide a patched old version, before my fix. To double check that tab was potentially allowing to steal the data)

Comment 75

[Daniel Bodea [:danibodea]]

Due to the high level of technicality and the fact that is quite hard to reproduce or verify (as mentioned in comment 72), I will be removing the qe+ flag. However, if verification of this report is necessary, please provide us with the necessary custom builds and information needed to complete.
Thank you.

Comment 76

[Artur Iunusov]

I can perform any test that required

Comment 77

[Artur Iunusov]

Follow up bug (enhancement): https://bugzilla.mozilla.org/show_bug.cgi?id=1890569

Comment 78

[Jesse Schwartzentruber (:truber)]

Unless I'm misunderstanding, I think this should have remained marked fixed in 122 (based on comment 72).

Comment 79

[Ryan VanderMeulen [:RyanVM]]

I guess, though it didn't actually ride the trains until 126 (bug 1889331).