1809122 - (CVE-2023-28164) Drag-and-drop of a URL from inside a cross-origin iframe should not be allowed to drop in the same tab if the iframe get removed during dragging (cross-origin attacks and spoofs)

Status: VERIFIED FIXED

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

Description

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

+++ This bug was initially created as a clone of Bug #1794268 +++

The patch in bug #1794268 does fix the issue of dragging a link from cross-origin iframe and dropping it outside of the iframe, but if the iframe get removed during dragging, we still trigger a navigation.

Comment 1

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: index.html

Comment 2

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Hmm, I could reproduce the issue (i.e. page navigates) on Mac, but not Linux. I guess that is why I did not aware this in bug 1794268.
Hi Virgil, do you remember which platform did you use to verify in bug 1794268 comment #18? If you were not using the Linux, would you mind trying Linux? Thanks!

Comment 3

[Virgil Sangerean]

The used platform from which the videos were made was on a Ubuntu 20.4. Do you need any more information about it?

Comment 4

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1809122 - Track top-level window context in DragSession; r?smaug

Comment 5

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

On Linux, I found it is easier to reproduce on release bug, but not every time. Mostly happens when first time load the test page, if I reload the test page, I can not reproduce it. And I could not reproduce on debug build. It seems like it depends on timing, to reproduce the issue, it requires the source windowContext being discarded before IPC exchanges for DnD.

Comment 6

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

Track top-level window context in DragSession; r=smaug
https://hg.mozilla.org/integration/autoland/rev/28f3bd70dc506b2cdbf847b981b76431fff00ad3
https://hg.mozilla.org/mozilla-central/rev/28f3bd70dc50

Comment 7

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

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

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

For more information, please visit auto_nag documentation.

Comment 8

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Hi Virgil, mind verifying when you get a chance as I am not able to reproduce the issue on Linux in my environment? Thanks!

Comment 9

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #7)

:edgar, is this bug important enough to require an uplift?

Yeah, probably worth uplifting, but I would like to verify it on nightly first.

Comment 10

[Donal Meehan [:dmeehan]]

:edgar could you submit a beta uplift approval request when ready?
Next week is the final beta week for Fx111

Comment 11

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 9316425 [details]
Bug 1809122 - Track top-level window context in DragSession; r?smaug

Beta/Release Uplift Approval Request

• User impact if declined: Allowing drags link from across origins within the same top-level window makes it easy to fool users that that's what they're doing.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Load the test page and try to scroll the page down by dragging the scroll bar and then drop it.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk as the changes are straightforward.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9316425 [details]
Bug 1809122 - Track top-level window context in DragSession; r?smaug

Approved for 111.0b6

Comment 13

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ef1c0e3b3755

Comment 14

[Virgil Sangerean]

Reproduced with 110.b8 Firefox while using the initial index.html file from bug 1794268.

Issue is verified fixed in 111.0b6 Firefox and the latest 112.a1 version of Firefox Nightly.

Comment 15

[Donal Meehan [:dmeehan]]

:edgar could you add an esr uplift request when ready?
There are conflicts with esr102, so it will need a rebased patch

Comment 16

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 9316425 [details]
Bug 1809122 - Track top-level window context in DragSession; r?smaug

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Allowing drags link from across origins within the same top-level window makes it easy to fool users that that's what they're doing.
• User impact if declined: Allowing drags link from across origins within the same top-level window makes it easy to fool users that that's what they're doing.
• Fix Landed on Version: 112
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk as the changes are straightforward.

Comment 17

[Donal Meehan [:dmeehan]]

:edgar this patch has conflicts with esr102, could you attach a patch that grafts cleanly to esr102?

Comment 18

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: [ESR102] Bug 1809122 - Track top-level window context in DragSession; r=smaug

This patch should work.

Comment 19

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 9320101 [details]
[ESR102] Bug 1809122 - Track top-level window context in DragSession; r=smaug

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Allowing drags link from across origins within the same top-level window makes it easy to fool users that that's what they're doing.
• User impact if declined: Allowing drags link from across origins within the same top-level window makes it easy to fool users that that's what they're doing.
• Fix Landed on Version: 112
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk as the changes are straightforward.

Comment 20

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: [ESR102] Bug 1809122 - Track top-level window context in DragSession; r=smaug

Sorry, wrong patch.

Comment 21

[Donal Meehan [:dmeehan]]

Comment on attachment 9316425 [details]
Bug 1809122 - Track top-level window context in DragSession; r?smaug

Approved for 102.9esr.

Comment 22

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/9833cc243666

Comment 23

[Virgil Sangerean]

Issue was reproduced on 102.8esr.

Issue is no longer reproducible on 102.9esr.

Comment 24

[Tom Ritter [:tjr]]

Attachment: advisory.txt