Drag-and-drop may be used to steal content across domains

RESOLVED FIXED in mozilla14

Status

()

Core
Drag and Drop
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: Michal Zalewski, Assigned: Neil Deakin)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug)

Trunk
mozilla14
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 .x+)

Details

(Whiteboard: [sg:moderate])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Hi folks,

As discussed by several researchers in the past, it is fairly easy to use CSS overlays and onmousemove tracking to trick the user into selecting text in an invisible IFRAME pointing to trusted-application.com; and then have him drag-and-drop it into a container on the top-level, malicious page.

The canonical example of a method to exploit this is interacting with faux scrollbars; or performing drag-and-drop-like operations in a web game, or a drawing application. I discuss the specifics here:

http://lcamtuf.blogspot.com/2010/10/attack-of-monster-frames-mini.html

(Start from "Since then, the situation with framed content"...)

I have seen privately shared proof-of-concepts exploits for this, and have been shown at least one in-the-wild rogue Facebook application that operated in a similar way - although I don't have any pointers handy at this point.

The risk is weakly mitigated by X-Frame-Options, but only for certain classes of web apps, and only on an opt-in basis. This is probably insufficient. We're currently siding with a view that this is best prevented by implementing one of the following options:

1) Preventing drag-and-drop if source frame is non-same-origin with destination window / frame (cross-domain drag-and-drop between top-level windows is probably OK). I believe this is unlikely to break legitimate uses of the browser.

2) If this is not feasible or too risky, requiring a secondary gesture to complete such drag-and-drop. For example, when the content hovers over non-same-origin destination, we could display a hovering tooltip saying "Hold shift to drop selection here".

Cheers,
/mz

Comment 1

7 years ago
I'd like to know how https://developer.mozilla.org/en/Security/CSP/Default_CSP_restrictions in Firefox 4 mitigates this, if at all.

Updated

7 years ago
Whiteboard: [sg:moderate]

Updated

7 years ago
blocking2.0: --- → ?
(In reply to comment #0)
> the following options:
> 
> 1) Preventing drag-and-drop if source frame is non-same-origin with destination
> window / frame (cross-domain drag-and-drop between top-level windows is
> probably OK). I believe this is unlikely to break legitimate uses of the
> browser.

I think this is entirely reasonable, for what it's worth.

(In reply to comment #1)
> I'd like to know how
> https://developer.mozilla.org/en/Security/CSP/Default_CSP_restrictions in
> Firefox 4 mitigates this, if at all.

I don't think CSP really helps us here.  CSP restricts where content can load from and prevents inline scripts from running, but it would be the attacker page's script that would do stuff with the dropped content so he wouldn't opt-in to CSP, and for the target page CSP does nothing to prevent text selection.
Hmm, nsContentAreaDragDrop seems to be adding every piece of drag data with the node's associated principal, so we should be able to do a cross-domain check before dispatching drop events to the target node, right?

Comment 4

7 years ago
Yeah, I think we could do that pretty easily.
Neil would know better, though.
(Assignee)

Comment 5

7 years ago
(In reply to comment #0)

> 1) Preventing drag-and-drop if source frame is non-same-origin with destination
> window / frame (cross-domain drag-and-drop between top-level windows is
> probably OK). I believe this is unlikely to break legitimate uses of the
> browser.

Do you mean prevent a drop when the data being dragged was from somewhere specifically loaded in a child frame (as opposed to something not loaded in a frame)?
(Reporter)

Comment 6

7 years ago
Yes, specifically loaded in a child frame that is not same origin with the receiving context (window / frame).

Drag-and-drop across top-level documents in different domains is probably fine.
(In reply to comment #6)
> Yes, specifically loaded in a child frame that is not same origin with the
> receiving context (window / frame).
> 
> Drag-and-drop across top-level documents in different domains is probably fine.

I don't think so.  Why?  The only difference there is that spoofing won't really work.
(Reporter)

Comment 8

7 years ago
Well, with drag-and-drop across top-level windows, the user knows what the origin and the destination are. When dragging something out of an IFRAME, this is not evident. That said, if you want to nuke both options, fine by me...
(Reporter)

Comment 9

7 years ago
You may be interested to know that a similar change is now committed in WebKit:

http://trac.webkit.org/changeset/71925
Blocking on this until it's figured out.
blocking2.0: ? → final+
blocking2.0: final+ → .x
Blocks: 639796
Component: Security → Drag and Drop
Product: Firefox → Core
QA Contact: firefox → drag-drop
Version: 3.6 Branch → unspecified
(Assignee)

Comment 11

6 years ago
Created attachment 576355 [details] [diff] [review]
patch

This patch:

- changes nothing for privileged callers
- if a drop occurs on a page, but the source of the drag is from a descendant frame, a principal comparison is made before allowing access to the dropped data.
- data urls are excepted from this. The webkit patch seems to have this check.
- prevents changing the source drag target to something from a different window.

Let me know if this isn't actually what is expected of this bug. The patch is more complex than I'd like so if a simpler check is possible, that would be good.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
(Assignee)

Comment 12

6 years ago
I wrote a manual test for this which I can attach to the bug if desired.
(Assignee)

Comment 13

6 years ago
Test builds are at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@mozilla.com-4cfbdd77a436/
Hey Neil, any chance I can get you to push this to Try again?  My contacts at Facebook said the builds you made didn't fix the testcase they were using (a copy of which I now have). I'm happy to dig in a bit if you'd be so kind as to re-push the patch to try.  Thanks.
(Assignee)

Comment 15

5 years ago
Can you provide the testcase?
> - data urls are excepted from this. The webkit patch seems to have this check.
What about blob urls?
(In reply to Neil Deakin from comment #15)
> Can you provide the testcase?

I'll email it to you.  They don't want the testcase to be public.
(Assignee)

Comment 18

5 years ago
OK, so this bug is expecting even privileged callers to be blocked from getting the data?

Right now the patch is using nsContentUtils::GetWindowFromCaller() to get the target window, but that, I assume, may not actually be the window that was dropped on. Is there a way to get the target of the drop event from nsDOMDataTransfer::MozGetDataAt where we don't have access to the event? Or perhaps it would be better to check for subframes earlier when we initialize?

Comment 19

5 years ago
Any updates on this? You can still drag HTML content from an iframe to the parent page (which is on a different origin). If this fix is proving to be difficult, perhaps you can take the another approach in the mean time, like blocking iframe view-source (Webkit/Chrome/Safari do this)?
What does view-source have to do with this?  That's the first mention of view-source in this bug....

Comment 21

5 years ago
view-source makes cross-origin drag-n-drop more exploitable, allowing for the theft of CSRF tokens or other sensitive elements in the DOM. Nearly every cross-origin drag-n-drop PoC I have seen makes use of view-source, ex: http://blog.kotowicz.net/2011/07/cross-domain-content-extraction-with.html.

Comment 22

5 years ago
Updates or thoughts on the view-source approach or the initial fix? Thanks.

Comment 23

5 years ago
Ping Boris, Neil, Michal - Thanks
I think blocking cross-domain drags makes more sense than blocking view-source in subframes, personally....  it may also be simpler to implement.
(Assignee)

Comment 25

5 years ago
Created attachment 596035 [details] [diff] [review]
Patch 2

This patch does the same as the previous but also prevents privileged callers from retrieving the dropped data being dropped onto a content page. This handles dropping on an editable area. It does not change any behaviour when dropping onto a chrome docshell.
(Assignee)

Updated

5 years ago
Attachment #596035 - Flags: review?(bugs)
(Assignee)

Comment 26

5 years ago
Note that the editor portions of the patch will need to be changed for 499008 in the future, and the recently checked in bug 707382 will affect the patch slightly on any branches that don't have it.
(Reporter)

Comment 27

5 years ago
View-source: is convenient, but there are many ways to achieve roughly the same effect (rich text, including forms and secret-bearing links, can be dragged and dropped, too). IIRC, Chrome disallows cross-origin drag and drop for a longer while, with no issues.
Comment on attachment 596035 [details] [diff] [review]
Patch 2


>+nsContentUtils::CheckForSubFrameDrop(nsIDragSession* aDragSession, nsDragEvent* aDropEvent)
>+{
>+  nsCOMPtr<nsIDocShellTreeItem> tdsti;
>+  nsCOMPtr<nsIContent> target = do_QueryInterface(aDropEvent->originalTarget);
>+  if (target && target->OwnerDoc()) {
>+    nsCOMPtr<nsIWebNavigation> twebnav = do_GetInterface(target->OwnerDoc()->GetWindow());
>+    tdsti = do_QueryInterface(twebnav);
>+  }
>+
>+  if (!tdsti)
>+    return true;
if (expr) {
  stmt;
}
Same also elsewhere.


>+
>+  PRInt32 type = -1;
>+  if (NS_FAILED(tdsti->GetItemType(&type)))
>+    return true;
>+
>+  // Always allow dropping onto chrome shells.
>+  if (type == nsIDocShellTreeItem::typeChrome)
>+    return false;
>+
>+  // If there is no source node, then this is a drag from another
>+  // application, which should be allowed.
>+  nsCOMPtr<nsIDOMDocument> sourceDocument;
>+  aDragSession->GetSourceDocument(getter_AddRefs(sourceDocument));
>+  nsCOMPtr<nsIDocument> doc = do_QueryInterface(sourceDocument);
>+  if (doc) {
>+    // Allow data uris to be used regardless.
>+    bool isDataURI = false;
>+    nsIURI* sourceURI = doc->GetDocumentURI();
>+    if (sourceURI)
>+      sourceURI->SchemeIs("data", &isDataURI);
>+
>+    if (!isDataURI) {
I don't understand why data url can be skipped.

>+      do {
>+        nsCOMPtr<nsIDocShellTreeItem> parentDsti;
>+        sdsti->GetParent(getter_AddRefs(parentDsti));
>+        sdsti.swap(parentDsti);
>+
>+        if (tdsti == sdsti) {
>+          // The drag is from a child frame.
>+          return true;
>+        }
>+      } while (sdsti);
Perhaps use nsIDocument::GetParentDocument(). It is faster and easier to use.

>+
>+  // Make sure that we only allow setting the element to something from
>+  // the same window.
Why this limitation? Wouldn't a principal check work here?
Attachment #596035 - Flags: review?(bugs) → review-
(Assignee)

Comment 29

5 years ago
(In reply to Olli Pettay [:smaug] from comment #28)
> I don't understand why data url can be skipped.

I think webkit had such a check, but I'll remove this.


> >+  // Make sure that we only allow setting the element to something from
> >+  // the same window.
> Why this limitation? Wouldn't a principal check work here?

I think I added this because calling addElement will change the source document, allowing one to override the frame checks by simply popping up another window and using a node from there.

Comment 30

5 years ago
Hey Neil, any chance we can get the latest diff up for review/commit?
(Assignee)

Comment 31

5 years ago
Created attachment 605513 [details] [diff] [review]
updated patch 3
Attachment #576355 - Attachment is obsolete: true
Attachment #596035 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #605513 - Flags: review?(bugs)
Comment on attachment 605513 [details] [diff] [review]
updated patch 3


>+  nsCOMPtr<nsIPrivateDOMEvent> privateEvent(do_QueryInterface(aDropEvent));
>+  nsDragEvent* dragEventInternal = static_cast<nsDragEvent *>(privateEvent->GetInternalNSEvent());
>+  if (nsContentUtils::CheckForSubFrameDrop(dragSession, dragEventInternal))
>+    return NS_OK;
>+

if (expr) {
  stmt;
}
Attachment #605513 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/3b64dbdc8d39
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Version: unspecified → Trunk
Comment on attachment 605513 [details] [diff] [review]
updated patch 3

Review of attachment 605513 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsContentUtils.cpp
@@ +5255,5 @@
>  bool
> +nsContentUtils::CheckForSubFrameDrop(nsIDragSession* aDragSession, nsDragEvent* aDropEvent)
> +{
> +  nsCOMPtr<nsIContent> target = do_QueryInterface(aDropEvent->originalTarget);
> +  if (!target && !target->OwnerDoc()) {

Consider what happens if target is indeed null. Also, OwnerDoc never returns null.

::: content/events/src/nsDOMDataTransfer.cpp
@@ +653,4 @@
>    if (mReadOnly)
>      return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
>  
>    mDragTarget = do_QueryInterface(aElement);

This QI isn't necessary
Attachment #605513 - Flags: feedback-

Comment 35

5 years ago
Ping - Any updates? Thanks, Facebook

Comment 36

5 years ago
NVM, mozilla14

Updated

5 years ago
Depends on: 775110

Updated

4 years ago
Depends on: 845194

Updated

4 years ago
Depends on: 851674
You need to log in before you can comment on or make changes to this bug.