1595399 - Spam many alerts without the 'Prevent this page from creating additional dialogs' checkbox

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[nathan]

It is possible to execute JavaScript code in the Firefox browser which causes an infinite loop of alerts that cannot be dismissed.
It removes the 'Prevent this page from creating additional dialogs' checkbox which would otherwise prevent this sort of thing.

The bug is working in the latest version of FireFox Developer Edition 71.0b8 (64-bit).
(I'm running MacOS 10.13.5 if that helps!)

By using the 'ondragend' inline event handler, an infinite loop of alerts can be created.
eg. <p id="test" draggable="true" ondragend="alert('Infinite alerts')">Please drag me!</p>

As soon as the user drags the element, an alert will be created. However, as soon as they dismiss it a new alert will be created.
The ondragend event seems to be re-triggered indefinitely, and bypassing the normal 'Prevent this page from creating addition dialogs' option.

Comment 1

[:Gijs (he/him)]

When I try the testcase, I can close the tab fine while the alerts are up, using the [x] on the tab or a shortcut. Do you see something else?

Comment 2

[nathan]

Yes, it is possible to close the tab.
It is simply the missing 'Prevent this page from creating additional dialogs'.

Comment 3

[:Gijs (he/him)]

Something is wonky in DOM land here. We queue up on the order of several 100 prompts in DOM code, and every time we check if the last time we closed a dialog was recent the time is still null. It seems like there's either some kind of re-entrancy (which might be its own bug?), and/or we should be setting mLastDialogQuitTime again as soon as we commit to displaying a prompt (ie after reading this value, perhaps as a base we should set the "last quit time" to the "current open time", as it were, to avoid issues where we re-enter and haven't yet set the last quit time.

On a debug build, I also hit an assertion in the window watcher:

Assertion failure: XRE_IsParentProcess(), at path/to/m-c/toolkit/components/windowwatcher/nsWindowWatcher.cpp:702
#01: nsWindowWatcher::OpenWindow(mozIDOMWindowProxy*, char const*, char const*, char const*, nsISupports*, mozIDOMWindowProxy**)[NightlyDebug.app/Contents/MacOS/XUL +0x590ddfc]

presumably from the window.open call that is also in the testcase linked at the URL. That seems... not good. Andreas, can you take a look?

Comment 4

[Daniel Veditz [:dveditz]]

A page can block itself, but it's not harming the browser in any other way. Does this need to be hidden as a security bug? It's clearly broken though!

Comment 5

[Andreas Farre [:farre]]

I tried this through mozregression and I've found a couple of things. First I landed at:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8de293399b5af8933eefeb4deea308c9f4707622&tochange=402db246b36ea3db0b784e7db75e68d63bddfb07

but this only tells us that dragging empty data is now supported, so I changed it to an <img>. Then I found that this has always been the case as far back as middle of 2017, where it instead of infinity-alerting actually crashed.

I'll continue digging some more tomorrow.

Comment 6

[Andreas Farre [:farre]]

So I've found and solved it. The problem was that entering modal state tries to end a currently active drag session. If ending a drag session triggers entering modal state we get a recursively triggering ondragend. Added a bool to check, not sure if that's the best. Will ask :smaug for review.

Comment 7

[Andreas Farre [:farre]]

Attachment: Bug 1595399 - Prevent EndDragSession from being called recursively. r=smaug!

nsGlobalWindowOuter::EnterModalState will try to end the current drag
session, so we need to keep track if we're entering modal state from
an event firing from ending the drag session.

Comment 8

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

https://hg.mozilla.org/integration/autoland/rev/ec2d5fef8d14d262525c7eecd7ec8d124bbd40c6
https://hg.mozilla.org/mozilla-central/rev/ec2d5fef8d14

Comment 9

[Julien Cristau [:jcristau]]

Does this need uplift or can it ride the trains to 73?

Comment 10

[Daniel Veditz [:dveditz]]

Unfortunately this bug does not meet the security bug bounty criteria.

Comment 11

[:Gijs (he/him)]

(In reply to Julien Cristau [:jcristau] from comment #9)

Does this need uplift or can it ride the trains to 73?

:farre, can you make a call here about uplifting this to beta?

Comment 12

[Daniel Bodea [:danibodea]]

This issue was reproduced on Firefox Release v72.0 and the fix was verified on Beta v73.0b2 and Nightly v74.0a1 on Windows 10 and Ubuntu 18.

Comment 13

[Andreas Farre [:farre]]

Comment on attachment 9110193 [details]
Bug 1595399 - Prevent EndDragSession from being called recursively. r=smaug!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Drag and drop is broken
• User impact if declined: Drag and drop is broken
• Fix Landed on Version: 73
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Change is small and is similar to the solution for ending a drag.
• String or UUID changes made by this patch: None

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9110193 [details]
Bug 1595399 - Prevent EndDragSession from being called recursively. r=smaug!

Fixes a case where drag and drop can be pretty broken. Approved for 68.5esr.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/9b3206de5f3f

Comment 16

[Daniel Bodea [:danibodea]]

I do now know how this fix should be verified because, in ESR68, this example paragraph that reproduces the issue is not draggable at all like it is in the 3 other channels so this issue cannot be reproduced, nor verified on ESR channel.

How should this be addressed further?

Comment 17

[Andreas Farre [:farre]]

So anything that is draggable should be able to reproduce this. So if you switch to an dragging an <img> it should repro. Tell me if it is not so and I will try to find another way.

Comment 18

[Daniel Bodea [:danibodea]]

I have modified the HTML test page to make an image draggable and with an alert on drag.
This issue is also verified in ESR v68.5.0esr. Thank you.