1828429 - .URL Extension Download Via Drag and Drop

Status: RESOLVED DUPLICATE of bug 291640

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

Description

[Ameen Basha M K]

Attachment: second.html

Hi team, Downloading .url file is restricted in Firefox on multiple phase Actual Download + Extension API's etc.
Because It has the impact of NTML Hash Leak, System Environment Variable Leak, Local File Read (Similar to symlink)

Have found a way to download the .url file via Drag and Drop.

Steps to reproduce:

1. Open the attached POC HTML File
2. Drag and Drop the safe <a> attribute to any location (You can find now the .url is downloaded)
3. Now for POC again upload it in the HTML file via browse,
4. BOOM you can find system.ini file from your local machine

Prevent .url creation (or) Creating .url file from drag and drop should be in proper http regex format to avoid including UNC path / File location instead of URL.(Since the user isn't aware of the background of drag and drop)

Sorry for my poor HTML without CSS :(

Tested in latest version of Firefox (112) on Windows 10 (Latest Patch)

Refer: CVE-2023-25734 for impact

Comment 1

[Ameen Basha M K]

I have attached a sample html file For CMD Invoke. Once the user clicks the drag and dropped file it will open the CMD (it has ability to invoke Executables) But user might not aware of this Since this is just a drag and drop of url from a website.

Comment 2

[Ameen Basha M K]

Attachment: opencmd.html

Comment 5

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

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

For more information, please visit BugBot documentation.

Comment 6

[:Gijs (he/him)]

As far as I can tell the straightforward part of the issue is that windows (and presumably linux/cocoa) implementations of the native drag service don't bother checking the requesting principal before going "sure, drag this link somewhere" (and create a .url or similar file for it, if asked for a file flavour/transferable/thingy), once the drop happens.

But fundamentally, I'd expect such a check to happen when the drag starts and for the URL/link/file flavour to not be available for such drags.

Fixing that requires changing drag innards.

So I looked at at least wallpapering this in the Windows widget implementation with a judicious call to the script security manager's CheckLoadURIStrWithPrincipal. That didn't help. Then I did some more debugging and found out that we set the requesting principal for the drag to system principal, which seems Bad.

From what I can tell, this is because we call nsBaseDragService::InvokeDragSession with aDOMNode set to the <browser> node. The stack at that point looks like this:

#05: nsBaseDragService::InvokeDragSession (C:\dev\mozilla-unified\widget\nsBaseDragService.cpp:387)
#06: nsBaseDragService::InvokeDragSessionWithRemoteImage (C:\dev\mozilla-unified\widget\nsBaseDragService.cpp:475)
#07: mozilla::EventStateManager::DoDefaultDragStart (C:\dev\mozilla-unified\dom\events\EventStateManager.cpp:2459)
#08: mozilla::EventStateManager::GenerateDragGesture (C:\dev\mozilla-unified\dom\events\EventStateManager.cpp:2231)
#09: mozilla::EventStateManager::PreHandleEvent (C:\dev\mozilla-unified\dom\events\EventStateManager.cpp:784)
#10: mozilla::PresShell::EventHandler::DispatchEvent (C:\dev\mozilla-unified\layout\base\PresShell.cpp:8228)
#11: mozilla::PresShell::EventHandler::HandleEventWithCurrentEventInfo (C:\dev\mozilla-unified\layout\base\PresShell.cpp:8197)
#12: mozilla::PresShell::EventHandler::HandleEventWithTarget (C:\dev\mozilla-unified\layout\base\PresShell.cpp:8104)
#13: mozilla::PointerEventHandler::DispatchPointerFromMouseOrTouch (C:\dev\mozilla-unified\dom\events\PointerEventHandler.cpp:628)
#14: mozilla::PresShell::EventHandler::DispatchPrecedingPointerEvent (C:\dev\mozilla-unified\layout\base\PresShell.cpp:7327)
#15: mozilla::PresShell::EventHandler::HandleEventUsingCoordinates (C:\dev\mozilla-unified\layout\base\PresShell.cpp:7125)
#16: mozilla::PresShell::EventHandler::HandleEvent (C:\dev\mozilla-unified\layout\base\PresShell.cpp:6944)
#17: mozilla::PresShell::HandleEvent (C:\dev\mozilla-unified\layout\base\PresShell.cpp:6887)
#18: nsViewManager::DispatchEvent (C:\dev\mozilla-unified\view\nsViewManager.cpp:677)
#19: nsView::HandleEvent (C:\dev\mozilla-unified\view\nsView.cpp:1150)
#20: nsWindow::DispatchEvent (C:\dev\mozilla-unified\widget\windows\nsWindow.cpp:4273)
#21: nsBaseWidget::ProcessUntransformedAPZEvent (C:\dev\mozilla-unified\widget\nsBaseWidget.cpp:1095)
#22: nsBaseWidget::DispatchInputEvent (C:\dev\mozilla-unified\widget\nsBaseWidget.cpp:1272)
#23: nsWindow::DispatchMouseEvent (C:\dev\mozilla-unified\widget\windows\nsWindow.cpp:4637)
#24: nsWindow::ProcessMessageInternal (C:\dev\mozilla-unified\widget\windows\nsWindow.cpp:5820)
#25: nsWindow::ProcessMessage (C:\dev\mozilla-unified\widget\windows\nsWindow.cpp:5091)
#26: nsWindow::WindowProcInternal (C:\dev\mozilla-unified\widget\windows\nsWindow.cpp:5043)
#27: CallWindowProcCrashProtected (C:\dev\mozilla-unified\xpcom\base\nsCrashOnException.cpp:32)
#28: nsWindow::WindowProc (C:\dev\mozilla-unified\widget\windows\nsWindow.cpp:4896)
#29: CallWindowProcW[C:\Windows\System32\USER32.dll +0xe858]
#30: DispatchMessageW[C:\Windows\System32\USER32.dll +0xe299]
(plus some more goop)

I'm pretty concerned by this because from looking at hg log for nsBaseDragService, it would seem that in bug 1809122 we added other code that depends on the drag session having information about the source window context and the toplevel source window context. But at least in the case in this bug, all that information is bogus because it is derived from aDOMNode which is the <browser>, not something in the child, and as a result the window context and toplevel window context are all browser.xhtml, rather than parent-process references to the content window context in which the drag originated.

Edgar/Olli/Emilio (whoever has time to look at this first / knows what is happening here), as you appear to have been the last people touching this code, and I am kind of a stranger in these lands: am I missing something? How is this supposed to work? ISTM that the parent process's notion of the drag should be using the principal associated with the document from which the dragged node originated, which AIUI is available in the parent through the usual browsingcontext/windowcontext IPC synced field goop. But we don't appear to be using it. Is there a reason for that (or a non-sec bug tracking fixing that), and/or am I barking up completely the wrong tree?

(searchfox coverage data, fwiw, also seems to suggest the windows drag service code is never hit in automation, and the lack of coverage for this remote path also looks... odd.)

Comment 7

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

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

But fundamentally, I'd expect such a check to happen when the drag starts and for the URL/link/file flavour to not be available for such drags.

I don't understand this. You would block most of the dnd coming from the web pages?

Comment 8

[Emilio Cobos Álvarez [:emilio]]

You can check URI etc in DragDataProducer, around here, fwiw.

Comment 9

[:Gijs (he/him)]

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

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

But fundamentally, I'd expect such a check to happen when the drag starts and for the URL/link/file flavour to not be available for such drags.

I don't understand this. You would block most of the dnd coming from the web pages?

I would block https://foo pages having a draggable URL flavour with URL file:/// - they cannot navigate to the URL, so I don't see why we should support dragging such a link. We certainly already do not support dropping the URL on our own UI (if you drop in the tabstrip, nothing happens, there's an error in the browser console indicating we blocked the drop because of security reasons, from the nsIDroppedLinkHandler). We could still support dragging the plain text of the link, but we shouldn't expose the URL flavour. Is that not feasible for some reason I'm missing? (very possible!)

I could also live with only suppressing this on drop, though it'll mean adding security checks on each platform and hoping that any subsequent implementations remember to add the same security checks (unless we can do it in the base drag service, but I'm not sure that level of indirection exists right now for OS outside-of-Firefox drops).

Comment 10

[:Gijs (he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

You can check URI etc in DragDataProducer, around here, fwiw.

Does that run in the content process? It sort of felt like this was meant to be funneled through this bit that I linked earlier, but that has no coverage, and I don't know if that means there's just no tests or if it's unrelated code, or if there's some other way this is meant to make its way to the parent process?

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Yes, I think that should run in the content process. That's the only place where we have the link. I think the reason it shows no coverage is because we don't have native drag sessions in automation.

Comment 12

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

blocking file:/// urls from web page does sound good. I wonder what would be considered a web page. Perhaps we could just check the content process type, if it is for a web page, that should cover all sorts of various urls and principals.

Comment 13

[:Gijs (he/him)]

Truly, there is nothing new under the sun.

Comment 14

[Ameen Basha M K]

The status was changed to resolved and the duplicate bug id was very old.

Im sure this will not be a duplicate. Because the duplicate bug id was very old and the status was resolved. if the issue was resolved in the 291640 bug itself then it should not be reproduced in the latest version but still the latest version have this issues.

So i dont think this as duplicate. FYI: i dont have access to 291640 bug.

Can you guys have a look into this once again..

Comment 15

[:Gijs (he/him)]

(In reply to Ameen from comment #14)

The status was changed to resolved and the duplicate bug id was very old.

Im sure this will not be a duplicate. Because the duplicate bug id was very old and the status was resolved. if the issue was resolved in the 291640

It's not "resolved" as in "fixed", just "resolved" as in "it's not useful to keep a duplicate ticket around given we already heard about this issue". That's just the way bugzilla deals with things. If we had fixed the bug it would be marked resolved fixed (which will hopefully happen to the original bug 291640 once I can get a patch together, but hasn't happened yet.

Comment 16

[Ameen Basha M K]

Done. Will my current bug eligible for bounty. I have spent time on this and notified you the issue.

Comment 17

[:Gijs (he/him)]

(In reply to Ameen from comment #16)

Done. Will my current bug eligible for bounty. I have spent time on this and notified you the issue.

Bounty decisions are for the bounty committee, but given it's a dupe, I'm not sure. Hopefully Tom knows.

FWIW, this should now be fixed in today's nightly build.

Comment 18

[Tom Ritter [:tjr]]

Everything flagged with sec-bounty? gets reviewed by the committee. When the bounty committee meets, we will consider this for a bounty in accordance with the bounty guidelines, including around duplicate submissions.

Comment 19

[Daniel Veditz [:dveditz]]

ALthough this is duped to a really old bug that would normally make this bug ineligible for a bounty, it did prompt us to take a new look at that old bug and fix it. As a result we'll treat this as a "split bounty" that normally is used when two people file duplicates within 72 hours.

Comment 20

[Daniel Veditz [:dveditz]]

This is at the low end of sec-moderate because you can't cause us to add parameters to the shortcut, and I can't think of any windows programs that would be malicious just because you opened them.

Comment 21

[Ameen Basha M K]

Team on which version the fix will be released? Kindly share the CVE details too.

Comment 22

[Tom Ritter [:tjr]]

It has been fixed in Firefox 115 in Bug 291640 - it will be released July 4th and the CVE details will be public then.

Comment 23

[Ameen Basha M K]

Hi team CVE-2023-37203 is assigned for this, but my name is not acknowleged in the advisory page.

Kindly acknowledge my name in the CVE

Ref: https://www.mozilla.org/en-US/security/advisories/mfsa2023-22/#CVE-2023-37203

Comment 24

[Ameen Basha M K]

Team any update on this

Comment 25

[Tom Ritter [:tjr]]

In keeping with the split-bounty decision, I've added your name to the credit - sorry we didn't catch this during advisory writing, it was an unusual case. https://github.com/mozilla/foundation-security-advisories/commit/f996e6d30792a5f9af51286207aec7b40947a4c0