Closed Bug 1189814 (CVE-2015-4519) Opened 9 years ago Closed 9 years ago

Dragging and dropping image to <textbox> pastes final URL of image after redirects

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox40 --- wontfix
firefox41 --- verified
firefox42 --- verified
firefox43 --- verified
firefox-esr38 41+ verified
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- affected
b2g-v2.2r --- affected
b2g-master --- fixed

People

(Reporter: gerv, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: regression, sec-moderate, Whiteboard: [adv-main41+][adv-esr38.3+] can be used in critical attacks against certain sites.)

Attachments

(2 files, 1 obsolete file)

https://fetch.spec.whatwg.org/#atomic-http-redirect-handling says:

"Throughout the platform, redirects (a response whose status is one of 301, 302, 303, 307, and 308) are not exposed to APIs. Exposing redirects might leak information not otherwise available through a cross-site scripting attack."

However, if you have:

<img src="http://foo.com/">

and that URL redirects to "http://bar.com?secret=12345", and then that loads the image, and then if you drag-drop the image to a textbox, the final URL is dropped (and therefore then available to script) rather than the initial URL.

This has been combined with cookie bombing to make a POC to steal Bugzilla secure attachments - see bug 1179241. I talked with Anne VK and he said that whatever we do about cookie bombing, this is also a bug.

Gerv
Keywords: sec-moderate
Whiteboard: can be used in critical attacks against certain sites.
Not sure if netfuzzer was the first to notice and abuse this (did .mario (H.) also use this?), but worth considering for a separate bounty. Or we might consider it a necessary element of the attack in bug 1179241 and therefore part of a combined bounty for that exploit. Either way, flagging for a bounty so we don't forget about it later.
Flags: sec-bounty?
This looks to have been done intentionally via bug 1013211.

Maybe Mats can offer some feedback.
Flags: needinfo?(mats)
I don't have any info other than what is in the comments on that bug.
Note bug 1013211 comment 12 though about the MIME type being from the final
destination (PNG) rather than what the linked content is (text/html), so
there's some internal confusion to sort out there if you want to backout.
At the time, I saw it a security issue that the user would end up with
a HTML file locally when he thought he was saving an image.

Backing out will likely break saving Deviantart images as desktop wallpaper
again (unless the site has changed since then).
Flags: needinfo?(mats)
Anne: thoughts on comment 3? Is there a way to avoid the security problem but keep the DeviantArt-style usecase working?

Gerv
Flags: needinfo?(annevk)
It sounds like we should reveal a different URL to privileged code (e.g., the OS) than to the page. I'm not super familiar with the details of drag-and-drop unfortunately.
Flags: needinfo?(annevk)
So when we drop an image on the desktop we're really just dropping a URL and the OS has to fetch it again? What if it can't (no cookies or whatever)? We've already downloaded the image, why aren't we dropping a locally saved file on the desktop?

If we're dropping a file on the desktop then the URL shouldn't matter, and we can remember the original URL.
Flags: needinfo?(mats)
It works differently on each platform. On Mac, the OS supplies us with the destination folder when a drop occurs, and we use nsIWebBrowserPersist to save the uri there. On Windows, I believe the actual image is used.

But this bug is about dropping on a textbox, which will drop the image URL (as textboxes don't accept images). Prior to bug 1013211, this used imgIRequest::URI (non-redirected uri) but now it uses imgIRequest::currentURI.
Keep in mind that it isn't necessary to drop the content into a textbox, it can obtained also by using "ondragstart" + "event.dataTransfer.getData". 

Try this:
data:text/html,<img style="height:500px;width:300px;" src="http://foo.com" ondragstart="alert(event.dataTransfer.getData('text/plain'));"></img>
Thank you Mario. Replace "http://foo.com" with "http://google.com" to see why this is a problem. You get "http://www.google.com/?..." back which currently there is no way to get to.

Neil, we can leak ::URI to the textbox and dataTransfer. We can leak currentURI to the OS and non-web content. Would that still break sites?
This simply backs out the changes to dom/base/nsContentAreaDragDrop.cpp
from bug 1013211.  It should restore the previous behavior for DND.
The other parts of 1013211 needs to stay because there are other
code that uses the added methods now.

Builds for testing:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/mpalmgren@mozilla.com-11ac37c2d524

I briefly tested the STR in bug 1013211 in a local Windows debug build
with this patch and I couldn't reproduce it.  I'll do some more testing
later with the Opt builds.  AFAICT, the site is still using a 301 redirect
to a HTML page so it appears nothing have changed on the site.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=11ac37c2d524
Assignee: nobody → mats
Flags: needinfo?(mats)
Attachment #8650427 - Flags: review?(enndeakin)
Attached patch mochitest (DO NOT LAND UNTIL BUG IS PUBLIC) (obsolete) — — Splinter Review
Comment on attachment 8650427 [details] [diff] [review]
fix (backout the DND part from bug 1013211)

You may want to add a synthesizeMouse for the mouseup event to the test.
Attachment #8650427 - Flags: review?(enndeakin) → review+
OK, thanks.
Attachment #8650428 - Attachment is obsolete: true
I've tested the win32 Opt Try build on Windows 7 and it seems to work just fine.
I wasn't able to reproduce the original problem in bug 1013211 while saving 10 images.
I don't recall what my failure rate was at the time, but bug 1013211 comment 8
mentions "50/50".  So with some luck the original problem is just gone.
Comment on attachment 8650427 [details] [diff] [review]
fix (backout the DND part from bug 1013211)

Approval Request Comment
[Feature/regressing bug #]:1013211
[User impact if declined]:reveals redirect URL when drag-n-drop images
[Describe test coverage new/current, TreeHerder]:passed tests on Try, the attached testcase works locally
[Risks and why]: very low risk as it backsout the change in the DnD code, returning to previous behavior.  The risk is that bug 1013211 returns (saving images from deviantart.com) but it appears to work in my testing [a trunk build]. (Would be good if QA can verify that in Aurora/Beta builds too.)
[String/UUID change made/needed]:none
Attachment #8650427 - Flags: approval-mozilla-beta?
Attachment #8650427 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/f19d55717b0a

Sounds like this should probably have esr38/b2g37 approval requests as well?
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(mats)
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8650427 [details] [diff] [review]
fix (backout the DND part from bug 1013211)

Fix a moderate sec issue. Taking it.
Attachment #8650427 - Flags: approval-mozilla-beta?
Attachment #8650427 - Flags: approval-mozilla-beta+
Attachment #8650427 - Flags: approval-mozilla-aurora?
Attachment #8650427 - Flags: approval-mozilla-aurora+
Comment on attachment 8650427 [details] [diff] [review]
fix (backout the DND part from bug 1013211)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: reveals redirect URL when dragging images
Fix Landed on Version: trunk/aurora/beta
Risk to taking this patch (and alternatives if risky): very low risk as it backsout the change in the DnD code, returning to previous behavior.  The risk is that bug 1013211 returns (saving images from deviantart.com) but it appears to work in my testing [a trunk build]. (Would be good if QA can verify that in ESR builds too.)
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(mats)
Attachment #8650427 - Flags: approval-mozilla-esr38?
Attachment #8650427 - Flags: approval-mozilla-b2g37?
Blocks: 1179241
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify+
Group: core-security → core-security-release
Comment on attachment 8650427 [details] [diff] [review]
fix (backout the DND part from bug 1013211)

OK, let's take this for ESR38.
Attachment #8650427 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Kamil or Florin, can one of you have a look to verify this fix? Thanks!
Flags: needinfo?(kjozwiak)
Flags: needinfo?(florin.mezei)
Flags: needinfo?(florin.mezei)
I tried to reproduce the bug on an affected build (e.g. 40.0.3) using the steps from Bug 1013211, Comment 9, with no success. Is there anyone else that might be able to verify the fix on esr38 [1] and 41.0b8 [2]? I'd really appreciate it.


[1] https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-*
[2] https://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/41.0b8-candidates/build1/
Mats, going through the example via comment # 8 sufficient enough for verification? Example:

* using a m-c build (20150820030202)
** output -> https://www.google.ca/?gfe_rd=cr&ei=Wh7zVdi0EpLd8geX15zoDA&gws_rd=ssl

* using the latest m-c build:
** output -> http://google.com/

Maybe going through bug # 1013211 and making sure that it's correctly working as well?
Flags: needinfo?(mats)
(In reply to Kamil Jozwiak [:kjozwiak] from comment #26)
> Mats, going through the example via comment # 8 sufficient enough for
> verification? Example:

Yes, I believe this is the expected before/after results.

> Maybe going through bug # 1013211 and making sure that it's correctly
> working as well?

Yes, please.  File a new bug if you see a problem there.  Thanks!
Flags: needinfo?(mats)
Reproduced the issue using the following build:
- https://archive.mozilla.org/pub/firefox/nightly/2015-08-20-03-02-02-mozilla-central/

Went through verification using the following builds:
- https://archive.mozilla.org/pub/firefox/nightly/2015-09-14-03-02-05-mozilla-central/
- https://archive.mozilla.org/pub/firefox/nightly/2015-09-14-00-40-20-mozilla-aurora/
- https://archive.mozilla.org/pub/firefox/candidates/41.0b9-candidates/build1/
- https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-macosx64/1442054391/
- https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-linux64/1442054391/
- https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-win32/1442054391/

Test Cases: (using the poc mentioned in comment # 8)

- ensured that the alert didn't include the entire link when dragging the img from the poc
- ensured that dropping the img from the poc into textboxes didn't dump the entire link

* OSX 10.10.5 x64 -> PASSED 
** fx43 -> PASS
** fx42 -> PASS
** fx41 -> PASS
** fxESR38 -> PASS

* Ubuntu 14.04.3 x64 VM -> PASSED
** fx43 -> PASS
** fx42 -> PASS
** fx41 -> PASS
** fxESR38 -> PASS

Windows 10 x64 (VM) -> PASSED
** fx43 -> PASS
** fx42 -> PASS
** fx41 -> PASS
** fxESR38 -> PASS

Relating to Bug # 1013211, I couldn't reproduce the original issue using the steps outlined in bug # 1013211 comment # 9. However, I did go through each of the above OS's/channels and tried downloading the image about 100 different times into different directories and couldn't reproduce the original problem.
Whiteboard: can be used in critical attacks against certain sites. → [adv-main41+][adv-esr38.3+] can be used in critical attacks against certain sites.
Alias: CVE-2015-4519
Attachment #8650427 - Flags: approval-mozilla-b2g37?
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: