1872841 - (CVE-2024-8900) Can write to clipboard from beforeunload event handler if navigation is user-triggered

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Apip]

Attachment: firefox_bypass_copy_to_clipboard.mp4

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

Steps to reproduce:

I found a "copy to clipboard" protection error in the clipboard access permission prompt when there was a page that asked to automatically paste text to the user's clipboard.

Normally: when a page asks to automatically enter text into the user's clipboard a permission prompt will appear.

Bug: Unfortunately, the bug that I found can bypass it even though the prompt status is blocked, where the reload function in the browser makes it possible to bypass the copy to clipboard permission automatically without the user having to touch the attacker's website page and simply being in the address bar or something. Maybe the browser has made a patch so that it requires user interaction to be able to copy to the clipboard which is already focused on the page, but unfortunately, it seems that the reload function enters the entire browser panel including automatically focusing on the page without the user knowing, so if the user is forced to press reload even though the user not in page focus for fear of threats, instead of protecting myself, but the text was successfully copied.

1. Go to https://bug.omapip.my.id/bar.html
2. For Firefox you can click the reload button on the browser
3. Check your clipboard

Actual results:

Copy to clipboard protection bypassed

Expected results:

The browser should ensure Copy to the clipboard can run and a permission prompt appears when the user focuses on the page rather than the browser to avoid other functions being executed via the browser instead of the page.

Comment 1

[Apip]

Attachment: bar.html

Comment 2

[:Gijs (he/him)]

Attachment: Minimal testcase

Here's a minimal testcase instead of one with loads of comments in Indonesian + unnecessary event listeners.

This also shows that a beforeunload without user activation doesn't work the same way.

Comment 3

[Daniel Veditz [:dveditz]]

I'm not sure we've well defined the scope of user activation (usually we just go by time-since-interaction), but letting pages do user-activation-based actions in an unload seems generally abusable even apart from this case. But this might be specified behavior so I'm not sure of the rating here.

Comment 4

[Oliver Medhurst [:canadahonk]]

NIing myself too as I can look soon and may affect other UA work. Happy to take this if wanted.

Comment 5

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

Edgar would know better about the copy-paste permission handling.

Comment 6

[Edgar Chen [:edgar] (OOO)]

We don't support permission prompt, but always requires page to have valid transient activation. Reloading a page should not make it have a valid transient activation. We need to check why a valid transient activation is generated on the reloaded page.

Comment 7

[Oliver Medhurst [:canadahonk]]

From some initial testing locally:

Pressing enter in URL bar: No copy
Pressing Ctrl+R in chrome: No copy[1]
Pressing Ctrl+R in page: No copy[1]
Clicking page reload button (location.reload): Copy (expected?)
Clicking chrome reload button: Copy (!)

It is worth noting:

1. Clicking on the page to "focus" it/etc will trigger a user activation so you should wait 5s for it to expire. This can lead to false positives for [1]
2. When copying text we do not consume a user activation, just check if there is a valid transient activation at that moment.
3. When reloading, it seems like past user activation carries over until it has expired (after 5s). This seems odd but not sure of spec for this.

Comment 8

[:Gijs (he/him)]

(In reply to Oliver Medhurst [:canadahonk] from comment #7)

When reloading, it seems like past user activation carries over until it has expired (after 5s). This seems odd but not sure of spec for this.

Is the issue that the flag is set on the Window object per spec (WindowContext in our codebase) and for reloads (which are sort of same origin by definition) we reuse that? Not sure what changing that would break...

Anyway, I poked at why we set this flag by setting a breakpoint in the WindowContext DidSet code for this field.

C++ stack:

#0 0x00000001133b64d8 in mozilla::dom::WindowContext::DidSet(std::__1::integral_constant<unsigned long, 13ul>) at mozilla-unified/docshell/base/WindowContext.cpp:377
#1 0x0000000113402014 in auto mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::Apply(mozilla::dom::WindowContext*, bool)::'lambda'(auto)::operator()<std::__1::integral_constant<unsigned long, 13ul>>(auto) const [inlined] at builds/opt/dist/include/mozilla/dom/SyncedContextInlines.h:223
#2 0x0000000113401fe8 in void mozilla::dom::syncedcontext::FieldValues<mozilla::dom::WindowContext::BaseFieldValues, 27ul>::EachIndexInner<mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::Apply(mozilla::dom::WindowContext*, bool)::'lambda'(auto)&, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul, 10ul, 11ul, 12ul, 13ul, 14ul, 15ul, 16ul, 17ul, 18ul, 19ul, 20ul, 21ul, 22ul, 23ul, 24ul, 25ul, 26ul>(std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul, 10ul, 11ul, 12ul, 13ul, 14ul, 15ul, 16ul, 17ul, 18ul, 19ul, 20ul, 21ul, 22ul, 23ul, 24ul, 25ul, 26ul>, auto&&) at builds/opt/dist/include/mozilla/dom/SyncedContext.h:168
#3 0x00000001133b5ba4 in void mozilla::dom::syncedcontext::FieldValues<mozilla::dom::WindowContext::BaseFieldValues, 27ul>::EachIndex<mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::Apply(mozilla::dom::WindowContext*, bool)::'lambda'(auto)&>(auto&&) [inlined] at builds/opt/dist/include/mozilla/dom/SyncedContext.h:154
#4 0x00000001133b5b9c in void mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::EachIndex<mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::Apply(mozilla::dom::WindowContext*, bool)::'lambda'(auto)>(auto&&) [inlined] at builds/opt/dist/include/mozilla/dom/SyncedContext.h:128
#5 0x00000001133b5b9c in mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::Apply(mozilla::dom::WindowContext*, bool) at builds/opt/dist/include/mozilla/dom/SyncedContextInlines.h:218
#6 0x00000001133b5634 in mozilla::dom::syncedcontext::Transaction<mozilla::dom::WindowContext>::Commit(mozilla::dom::WindowContext*) at builds/opt/dist/include/mozilla/dom/SyncedContextInlines.h:134
#7 0x00000001133ab494 in mozilla::dom::WindowContext::SetUserActivationState(mozilla::dom::UserActivation::State) [inlined] at builds/opt/dist/include/mozilla/dom/WindowContext.h:105
#8 0x00000001133ab448 in mozilla::dom::WindowContext::NotifyUserGestureActivation() at mozilla-unified/docshell/base/WindowContext.cpp:487
#9 0x000000011075016c in mozilla::dom::Document::NotifyUserGestureActivation() at mozilla-unified/dom/base/Document.cpp:16783
#10 0x0000000110643870 in nsDOMWindowUtils::SetHandlingUserInput(bool, nsIJSRAIIHelper**) at mozilla-unified/dom/base/nsDOMWindowUtils.cpp:4053
#11 0x000000010f841b3c in _NS_InvokeByIndex at mozilla-unified/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_aarch64.S:74
#12 0x0000000110019e84 in CallMethodHelper::Invoke() [inlined] at mozilla-unified/js/xpconnect/src/XPCWrappedNative.cpp:1627
#13 0x0000000110019e70 in CallMethodHelper::Call() [inlined] at mozilla-unified/js/xpconnect/src/XPCWrappedNative.cpp:1180
#14 0x0000000110019498 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) at mozilla-unified/js/xpconnect/src/XPCWrappedNative.cpp:1126

plus a bunch more JS bits invoked from the actor manager's message handling "receive" code. (note: this is all in the child.)

JS callstack points to this weird bit:

https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/toolkit/modules/E10SUtils.sys.mjs#713-721

called from: https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/browser/actors/BrowserTabChild.sys.mjs#22-23

Seems Nika's comment there may have been prophetic...

Comment 9

[Oliver Medhurst [:canadahonk]]

Regarding past user activation for reloads, I filed a clarification spec issue. From that, it seems we should theoretically have a new Window object for refreshes (/etc). I'll likely file a separate public bug for this as mostly unrelated. (This may also affect more things than user activation generally?)

Comment 10

[:Gijs (he/him)]

(In reply to Oliver Medhurst [:canadahonk] from comment #9)

Regarding past user activation for reloads, I filed a clarification spec issue. From that, it seems we should theoretically have a new Window object for refreshes (/etc). I'll likely file a separate public bug for this as mostly unrelated. (This may also affect more things than user activation generally?)

We might already get a new window object (but not a new windowcontext one)? I don't know how to test this easily tbh. (If you've checked and it's the case that we reuse the window ref then that's fine... just clarifying that I didn't verify this, I was guessing.)

For the reload case, I do think we could try to stop using BrowserTabChild and organize the refresh from the parent process. This would be better for other reasons, but I don't know how hard it would be to change. That would fix this bug without changing Window/WindowContext/transient-user-activation lifetimes. For the latter, we could potentially also reset it if/when a new document loads in the window/windowcontext, I imagine, if making changes to window/windowcontext lifetime proves difficult.

Comment 11

[Sean Feng [:sefeng211]]

Marking this bug as S2 given this leaks information.

Comment 12

[Apip]

any update on this?

Comment 13

[:Gijs (he/him)]

(In reply to Apip from comment #12)

any update on this?

Any updates will appear in the bug, please do not spam all the bugs you file asking for updates - it doesn't help anybody.

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to :Gijs (he/him) from comment #10)

(In reply to Oliver Medhurst [:canadahonk] from comment #9)

Regarding past user activation for reloads, I filed a clarification spec issue. From that, it seems we should theoretically have a new Window object for refreshes (/etc). I'll likely file a separate public bug for this as mostly unrelated. (This may also affect more things than user activation generally?)

We might already get a new window object (but not a new windowcontext one)? I don't know how to test this easily tbh. (If you've checked and it's the case that we reuse the window ref then that's fine... just clarifying that I didn't verify this, I was guessing.)

For the reload case, I do think we could try to stop using BrowserTabChild and organize the refresh from the parent process. This would be better for other reasons, but I don't know how hard it would be to change.

Oliver pointed out that this is the code we want to try per this comment from Gijs - https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/browser/actors/BrowserTabChild.sys.mjs#22-23

Andreas, do you have ideas how hard it is to do the change?

That would fix this bug without changing Window/WindowContext/transient-user-activation lifetimes. For the latter, we could potentially also reset it if/when a new document loads in the window/windowcontext, I imagine, if making changes to window/windowcontext lifetime proves difficult.

Comment 15

[Andreas Farre [:farre]]

Gut feeling is that it would be fairly easy. If there isn't a method of doing this already on CanonicalBrowsingContext then it should be fairly easy to add.

Comment 16

[:Gijs (he/him)]

I'm a bit surprised that the outcomes described in comment #7 do not seem to match the description in bug 1876943. Maybe it's an OS difference? It'd be worth understanding that better...

Comment 17

[Apip]

Hi team, any update?

Comment 18

[Apip]

Hi team, any updates? Can I disclose this vulnerability to the public once it's fixed?

Comment 19

[:Gijs (he/him)]

(In reply to Apip from comment #18)

Hi team, any updates?

Any updates will appear in the bug. Please do not comment just to ask for updates.

Can I disclose this vulnerability to the public once it's fixed?

After a fix has rolled out, yes. But no fix has been created nor rolled out at this stage.

Oliver, do you have time to look into this more?

Comment 20

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to :Gijs (he/him) from comment #19)

(In reply to Apip from comment #18)

Hi team, any updates?

Any updates will appear in the bug. Please do not comment just to ask for updates.

Can I disclose this vulnerability to the public once it's fixed?

After a fix has rolled out, yes. But no fix has been created nor rolled out at this stage.

Oliver, do you have time to look into this more?

IIRC, Andreas plans to see if he can do https://bugzilla.mozilla.org/show_bug.cgi?id=1872841#c15 quickly. It's just that he has other urgent bugs before this - but this is next in his queue.

Comment 21

[Andreas Farre [:farre]]

Attachment: WIP: Bug 1872841 - Start reload in the parent process.

Comment 22

[Andreas Farre [:farre]]

The patch in comment 21 is doing what we talked about in comment 15. It seems to be very simple to do, but directly calling reload in the parent process. The correctness of this, with regards to activation on the other hand, I'm not super sure of. I'd like to try to land this in Bug 1886222, which I think is secret enough to not expose this issue, and then I'd be able to get some try runs in. With this change, then the behaviour of the last line in Oliver's comment 7

Clicking chrome reload button

does not trigger the clipboard path, since the reload is immediate. Does this sound reasonable?

Comment 23

[:Gijs (he/him)]

(In reply to Andreas Farre [:farre] from comment #22)

The patch in comment 21 is doing what we talked about in comment 15. It seems to be very simple to do, but directly calling reload in the parent process. The correctness of this, with regards to activation on the other hand, I'm not super sure of. I'd like to try to land this in Bug 1886222, which I think is secret enough to not expose this issue, and then I'd be able to get some try runs in. With this change, then the behaviour of the last line in Oliver's comment 7

Clicking chrome reload button

does not trigger the clipboard path, since the reload is immediate. Does this sound reasonable?

Yes, very reasonable. Doing a reload in browser chrome shouldn't trigger user activation in the child in a way that allows the webpage to do other things.

It looks like this was added in bug 1260102, so that calls in the child triggered by user interaction with e.g. context menus, were appropriately allowed. I think "only" changing the reload command to "just" use privileged APIs is fine to resolve this bug as-is.

That said, I think we'll need 2 follow-ups after that change lands:

1. it looks like "reload frame" is implemented without the wrapHandlingUserInput call, and ends up in https://searchfox.org/mozilla-central/rev/55944eaee1e358b5443eaedc8adcd37e3fd23fd3/browser/actors/ContextMenuChild.sys.mjs#143 . It should probably use a parent-process mechanism, like the "page reload" context menu (which already uses the XUL command so will be fixed by the patch you described). I imagine that "reload frame" suffers from the inverse problem where we do not treat the request to reload the frame as being user-activated which may affect whether it is successful / how it behaves. (Though I don't immediately recall why/how reloads differ if they do/don't have user activation, I'm taking it on trust that there's some kind of difference.)
2. it looks like we also use wrapHandlingUserInput for the media commands, cf. https://searchfox.org/mozilla-central/search?q=wrapHandlingUserInput&path=&case=false&regexp=false pointing to https://searchfox.org/mozilla-central/rev/55944eaee1e358b5443eaedc8adcd37e3fd23fd3/browser/actors/ContextMenuChild.sys.mjs#77 . So there's perhaps nominally a similar bug to this one where if you had a media control, encouraged the user to play its contents, you could then use the event generated by the action on the media to do something else that requires user activation. Honestly, that seems [even] more far-fetched than the reload case, because it would just be easier to convince the user to click in the page which would also grant transient user activation. Still, if there's some other mechanism by which we could do this so we can have our cake and eat it wrt the media control (that is, the command succeeds "as if" user-triggered, because it is, but also, the page/window is not granted transient user activation) then that would be interesting. But this doesn't seem like it'd need to be a security bug or a priority as such, unless I'm missing something.

I defer to Oliver/you wrt whether it's useful to have a separate investigation around how/where we store user activation and it "sticking around" post-reload. The comments / implementation around wrapHandlingUserInput kind of suggests that the expectation is that user activation is removed immediately once the desired action is carried out, cf. https://searchfox.org/mozilla-central/rev/55944eaee1e358b5443eaedc8adcd37e3fd23fd3/toolkit/modules/E10SUtils.sys.mjs#717-718,720 . This doesn't appear to be the case [anymore] ? Or perhaps only in the reload case because of the new window/doc? Or is this just a "the transient activation lasts 5s" thing? Anyway, may be worth investigating/fixing orthogonally to the proposed change.

Comment 24

[Andreas Farre [:farre]]

Me, :peterv, :jjaschke and :avandolder discussed this a bit yesterday, and one thing that is noteworthy is that though we shift the reload call to not go through an actor side channel we instead go through (I guess) the RemoteWebNavigation's implementation of reload, effectively calling CanonicalBrowsingContext.reload, and this will also send IPC to the content process initiating the reload. Which means that the "fix" here might be to not do wrapHandlingUserInput. In any case, this solution is better since we're consolidating our code paths.

Comment 25

[Daniel Veditz [:dveditz]]

Sorry for the burst of bugspam: filter on tinkling-glitter-filtrate
Adding reporter-external keyword to security bugs found by non-employees for accounting reasons

Comment 26

[Apip]

Hi team, are you sure this is S3?, for all this?

Comment 27

[Apip]

Hi Guys,

Sorry if it's too disturbing, I want to make sure if there is an update? for fixing this vulnerability?

Comment 28

[:Gijs (he/him)]

I think this was fixed by bug 1886222, as I can't reproduce in Firefox 130 beta (the fix landing in 129). Am I missing something?

Comment 29

[Apip]

(In reply to :Gijs (he/him) from comment #28)

I think this was fixed by bug 1886222, as I can't reproduce in Firefox 130 beta (the fix landing in 129). Am I missing something?

same, i can't reproduce on 129, But i didn't get the bounty and CVE notification

Comment 30

[Apip]

and strangely it turned out to have been fixed 2 months ago, what happened here?

Comment 31

[:Gijs (he/him)]

(In reply to Apip from comment #30)

and strangely it turned out to have been fixed 2 months ago, what happened here?

It got fixed by a code cleanup style patch that rode the trains, from a "normal" (ie public, non-security) bug.

The bounty/CVE questions I'll fwd to someone else...

Comment 32

[Andreas Farre [:farre]]

Yeah, :Gijs is correct, this was fixed by bug 1886222.

Comment 33

[Tom Ritter [:tjr]]

I have flagged it for bounty consideration and we will look at it and figure out the CVE implications as well

Comment 34

[Ryan VanderMeulen [:RyanVM]]

FWIW, bug 1886222 backports cleanly to ESR128 if we wanted to do so. ESR115 would require a bit more work.

Comment 35

[Apip]

any update?

Comment 36

[Daniel Veditz [:dveditz]]

need to create a 129 retroactive advisory for this one.

We missed the 128.2 train, but I don't think we need to fix this on ESR

Comment 37

[Apip]

Hi guys,

any updates for my CVE?

Comment 38

[Tom Ritter [:tjr]]

Sorry about that, I have assigned the CVE and updated the advisory

Comment 39

[Apip]

HI guys,

Can you tell me how long it will take for CVE to come out or release the latest version? and also how long the payment time usually is?

Comment 40

[Tom Ritter [:tjr]]

Sorry, I forgot to assign the CVE in Bugzilla. It is CVE-2024-8900 and is included here: https://www.mozilla.org/en-US/security/advisories/mfsa2024-33/

Bounty payment typically takes 3-5 weeks for it to land in your account. If you have further questions about the bounty please email [e-mail]

Comment 41

[Apip]

noted, thanks tom

Comment 42

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt