Only allow dragging images as images if the OS recognizes the extension as one of our image extension
13.66 KB, patch
|Details | Diff | Splinter Review|
15.59 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 By dynamicly creating a hybrid of a gif image and a batch file it is possible to create an executable file by dragging an image to the desktop. Reproducible: Always Steps to Reproduce: 1. Open http://www.mikx.de/firedragging/ 2. Drag the image to your desktop 3. Open the dropped file using double click Actual Results: The file foximg.bat gets created on dektop Expected Results: The file foximg.gif or a link to foximg.gif/foximg.php should be created. Usually Firefox prevents that an executable, non-image file gets directly dragged to the desktop (e.g. by supplying malware.exe as the src of an image tag). Instead Firefox creates a link to the file on the desktop. If you create a hybrid of a gif image and a batch file you can trick firefox (see PoC link). Since the file renders as a valid image (the gif parser seems to be pretty forgiving) firefox tries to copy the image to the desktop when dragged. If you create the image dynamicly and fore the content type image/gif the file can be of any extension (e.g. image.bat or image.exe). The windows batch file parser is also forgiving. It just ignores the first line of "gif trash" and execute whatever you append to the end of the hybrid file. The PoC creates the directory "c:\booom". Since windows hides known file extensions by default a user can only tell that something went wrong by looking at the file icon, which is different of course (depending on file association). If the user does not care or know what this different icon means a double click to view or edit the "image" he just dropped executes the batch file instead. Tested with Firefox 0.9.2, 1.0 and the nightly latest-trunk build from 2005-01-26 on Windws XP SP2. Files served from Apache 1.3 and PHP 5.
Not sure what to think about this one. The file on the desktop clearly has the batch file extension and icon, but most people won't notice either one. The fact that the file renders OK as an image might make it easier to convince someone to drag it. The most dangerous things about this--windows hides extensions by default, and especially that the batch executes despite the binary garbage--are pure windows problems we can't do anything about. The one thing we could do is not let folks drag images at all, let it drop on the desktop as a shortcut like dragging a link. That'll annoy some people, which is OK if it solves a problem, but I'm not sure it does. Interesting: despite the .bat extension if you drag it back onto the browser it renders as image/gif.
we could enforce an extension by content type when we do dragging. but we'll mess up and compound the problem :).
>Usually Firefox prevents that an executable, non-image file gets directly >dragged to the desktop [...] Instead Firefox creates a link to the file on the > desktop. probably windows does that, not ffox. try holding ctrl (I think. or was that shift?)
(In reply to comment #3) > probably windows does that, not ffox. try holding ctrl (I think. or was that shift?) proof-of-concept image crtl, shift and combinations do no change anything (binary gets dropped) on alt and drag windows asks me if i want to install an active desktop item. WTF? an img tag with malware.exe as src crtl, shift, alt and combinations do no change anything (link gets dropped)
i believe our handling of modifiers is a known bug, but i have to go and can't look until sunday. sorry.
Perhaps the drop code could check against the list of executable extensions and either block those to force an explicit Save As if the user really wants it.
jst, can you take a look at this one?
Assignee: bugs → jst
Flags: blocking-aviary1.0.1? → blocking-aviary1.0.1+
This makes gecko only support dragging of tags as images if the extension in the URI maps on the OS to one of the image types that gecko supports. Might break in edge cases, but the only alternative is to start making up extensions, and I kinda don't think we want to go there...
Comment on attachment 173232 [details] [diff] [review] Only allow dragging images as images if the OS recognizes the extension as one of our image extension sr=dveditz, patch works great.
Attachment #173232 - Flags: superreview?(dveditz) → superreview+
Comment on attachment 173232 [details] [diff] [review] Only allow dragging images as images if the OS recognizes the extension as one of our image extension >+ PRBool supportedType; >+ if (NS_FAILED(imgLoader->SupportImageWithMimeType(mimeType.get(), >+ &supportedType)) || (Random comment: ugh, why is that not SupportsImageWithMimeType? Oh well...) ^ r=caillon. Don't we need this for seamonkey too? Seems like this should move products, and be nominated to block 1.7.6 ...
Attachment #173232 - Flags: review?(caillon) → review+
It's on the 1.7.6 tracking bug, but you're right --> Core layout
Component: OS Integration → Layout
Product: Firefox → Core
Version: unspecified → 1.7 Branch
Comment on attachment 173232 [details] [diff] [review] Only allow dragging images as images if the OS recognizes the extension as one of our image extension Adding caillon's r=
(In reply to comment #8) > Created an attachment (id=173232)  > Only allow dragging images as images if the OS recognizes the extension as one > of our image extension Does this mean, that dynamicly generated images ending on .cgi/.php/.asp/.pl can't be dragged any more? What about urls containing no extension at all? This would affect a lot of websites and content management systems. Especially to Mac users dropping things to the desktop is quite common (since you have no "right mouse click" context menu with "save image as"). If so, would it be possible to add a whitelist (.gif/.jpg/.jpeg/.bmp/.png) and try to change an "invalid" extension to a valid extension based on the content type? This would allow us to still drag and drop a dynamicly generated gif image for example (dynimg.cgi gets dropped as dynimg.gif if content type is image/gif). BTW, can i test a patch somehow beside compiling firefox on my own? Is there a nightly build available containing this fix?
I think we need an answer to the last comment before checking this in.
on public website, known by press, opening.
Oops. Should've read more comments in this bug before I checked in, but I didn't... So this is now checked in, and yes, this will break dragging of images that are dynamically generated. Please test tomorrows nightly builds and file *new* bugs on coming up with a scheme for changing or making up file extensions for images loaded with URIs w/o a proper image extension.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Reporter objected, for now. closing again. (mess)
I filed bug 280947 on the issues this caused.
Since the bug is fixed i will make it public tonight. Just to let you now beforehand.
Comment on attachment 173232 [details] [diff] [review] Only allow dragging images as images if the OS recognizes the extension as one of our image extension a=asa for branches checkins.
Fixed on the 1.0.1 branch.
Has this landed on the 1.7 branch as well?
Whiteboard: [sg:fix] needs approval → [sg:fix] need 1.7.6 landing
Fix landed on the 1.7.6 branch.
Whiteboard: [sg:fix] need 1.7.6 landing → [sg:fix]
This was (is) assigned as Secunia ID 14160; http://secunia.com/advisories/14160/ (Mozilla / Firefox Three Vulnerabilities) marked as "The vulnerabilities have been fixed in the CVS repository" on 8th February, 2005.
Verified Fixed everywhere.
Status: RESOLVED → VERIFIED
This bug is reproducible on Firefox 1.0.2 / Mozilla 1.7.6 gtk2 build. Should we fix it?
(In reply to comment #28) > This bug is reproducible on Firefox 1.0.2 / Mozilla 1.7.6 gtk2 build. > Should we fix it? > Oh, sorry for my ignorance, in GNOME, it's just creating a link to the web location, so it's not an issue.
You need to log in before you can comment on or make changes to this bug.