Closed Bug 657462 (CVE-2011-2372) Opened 13 years ago Closed 13 years ago

Holding enter allows arbitrary code execution due to Download Manager

Categories

(Toolkit :: Downloads API, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla10
Tracking Status
firefox5 - wontfix
firefox6 - wontfix
firefox7 + wontfix
firefox8 + fixed
firefox9 + fixed
blocking1.9.2 --- .24+
status1.9.2 --- .24-fixed

People

(Reporter: marius.mlynski, Assigned: Gavin)

References

Details

(Keywords: verified1.9.2, Whiteboard: [sg:critical][qa!] semi-public :-( )

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

It is possible to execute an arbitrary code in the context of user's operating system if a user holds Enter for about 1-2 seconds. Consider the attached example -- a file download prompt pops up and a user has only about a second to react (with the default Firefox settings -- it could be a fraction of second more if the option to always ask where to save files was explicitly selected). If they keep holding Enter and there was no "Downloads" window on the task bar prior to the prompt (thus the newly appearing "Downloads" window with the malicious file highlighted gains focus), an arbitrary code will be executed. No user interaction other than holding Enter is needed for a successful exploitation of this issue. The windows appear in a very brief instant and an attacker can use a number of methods to easily distract user's attention to make them hold the crucial key a fraction of second longer.

I think that the primary problem here is that the keyboard buffer isn't maintained correctly when the download prompt gains focus. It should be flushed and no new keys should be registered until after all keys are released.

Reproducible: Always

Steps to Reproduce:
1. Open script.htm
2. Press and hold Enter.

Actual Results:  
calc.exe starts.

Expected Results:  
No file should be launched without an explicit consent from the user.

The attached example has been verified to launch calc.exe in Firefox 4.0.1 on Windows 7 SP1 64-bit and Windows XP SP2 32-bit.
Attached file Proof of concept. —
Component: Security → Download Manager
Product: Firefox → Toolkit
QA Contact: firefox → download.manager
Summary: Holding enter allows arbitrary code execution. → Holding enter allows arbitrary code execution due to Download Manager
Whiteboard: [sg:critical]
The problem here seems to be that we don't think we're "launching" anything, we're simply "opening" it in the System defined app handler. .jar is registered to Java, and locally opened Java applications (not applets) have extra powers.

At the very least we should add .jar to the list of executable extensions.

But .jar probably isn't the only one. What can local AIR apps do, for instance?

I'm surprised this dialog no longer seems to have a security dialog delay before the button gets enabled. Pretty sure it had one in the past because lcamtuf/mz was complaining it was too short.
Assignee: nobody → sdwilsh
Status: UNCONFIRMED → NEW
blocking1.9.2: --- → .18+
Ever confirmed: true
This is not just a case of what an external app can do by design -- I imagine that attackers can use this method to target vulnerabilities in other seemingly safe applications. I strongly believe that a proper fix to this issue should ensure that holding down Enter does not result in opening any kind of file when a download prompt suddenly appears. If you don't want to fiddle at the keyboard buffer level, you can set "Cancel" as the default button and (re-)implement a short delay before the OK button is enabled (such delay would also help to circumvent some clickjacking attacks).
Or we could have the download manager open only if it sees a keydown for enter followed by keypress (as opposed to just a keypress).
That won't help if the web site convinces you to press Enter over and over.
Assignee: sdwilsh → gavin.sharp
The problem really is two-fold:
a) .jar files aren't marked as executable on Windows when they apparently are (for some loose definition of "executable"), at least some of the time

b) the delay is currently hardcoded to 250ms - this is way too short to notice the dialog come up before the Enter takes effect and confirms the dialog.

Fixing a) is simple, though still potentially leaves Mac/Linux vulnerable if the handlers there are dangerous (I have no idea whether this is usually the case - the default handler on my Mac is just an archive utility but I think I changed that from the default).

b) is more annoying. We can bump the delay to something closer to the amount of time most people will take to perceive that the dialog opened (but still try to stay under the amount of time it takes them to react, to avoid annoying users actually trying to download something quickly). That's hard to get right.

We could also do something along the lines of Boris' comment 4 to mitigate this attack specifically, which would probably decrease the reliability of any attacks despite still being circumventable (per comment 5).
Depends on: CVE-2011-3666
There's no fix that's going to make Firefox 5 or 3.6.18 for this problem generically. We might get bug 662309 for .jar files specifically.
blocking1.9.2: .18+ → ?
This appears fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.18pre) Gecko/20110609 Namoroka/3.6.18pre (.NET CLR 3.5.30729). If I run the attached testcase and hold enter, I briefly see the flash of a download prompt dialog which is immediately dismissed. No calculator opens.

On 1.9.17, the same prompt then opens up calculator.

So something we checked in just mitigated this exact scenario.
Maybe bug 662309 was enough of a change to break this specific testcase (if not the general case)?
bug 662309 fixed this one testcase on windows. didn't fix it on Mac, didn't fix the potential to find other exploitable helper apps.
blocking1.9.2: ? → .19+
Depends on: 663899
Gavin, Johnathan, we need to do something here pretty soon, who's the best person for this?
This bug is at the "need to decide what to do" stage, not "have someone do it".

A full-on delay (as in the add-on install prompt) is going to frustrate users. Delaying only when the default action is "open" might be more acceptable, I suppose, but maybe we should just never have the default action be "open". I think that matches what other browsers do.
Gavin, who will make that decision?
If we make "Save" the default option can the "Open" key be activated by pressing "o" (or some other key)? If so that doesn't really help us. The button still needs to be accessible, but Tab{+}-space should be safe enough.

Doesn't help the people who automatically open content of certain types, but that's simply a dangerous thing to do.
// ignore me: testing why Mariusz isn't getting BMO mail for comments
(In reply to comment #14)
> If we make "Save" the default option can the "Open" key be activated by
> pressing "o" (or some other key)? If so that doesn't really help us. The
> button still needs to be accessible, but Tab{+}-space should be safe enough.

It won't be possible to activate the button with a single keypress. It will always be true that a combination of keypress (access key or tabbing) will allow access to the button, but that's unavoidable.
I investigated this further last week, and it turns out that we already default to "Save" in the common case. It's only once you've selected "Open with..." for a given file type and selected "Always do this" that we end up defaulting to "open" being selected, AFAICT.

Mariusz, is that consistent with you've seen?
No, unless the behaviour has changed between 4.0.1 and 5.0. Here's how it worked on Windows 7/XP the last time I investigated the issue:

1. When I created a fresh user profile, the download manager would always default to "Open with" for each specific filetype.
2. The download manager would remember which option was selected the last time the file of a specific type was accessed and would default to this option, unless the filetype has not been downloaded since the user profile was created, in case of which it would default to "Open with" as per point 1.
3. Things are still not safe if "Save File" is selected. It takes a little longer, but after about a second the box with the downloaded files list pops up, and since the downloaded file is focused, it will be launched. A large number of instances of the downloaded file will be opened if you keep holding enter for too long.
4. The only case where the file is not executed is when both "Save File" is selected and the box with the downloaded files happens to be on the taskbar (ie. you have downloaded something earlier and you didn't close that box). The reason is that the box is not brought to the front of the screen in this particular case, so holding enter will only allow to download the file to the user's directory but not to launch it.

I will try to re-test this with the latest version.
There shouldn't be any behavior differences in any moderately recent (>3.0?) versions of Firefox - I'm pretty sure we haven't changed this code in a while.

Which file types were you testing? That might matter.
I don't think this is a change that we can take for beta, especially since we don't have a fix yet.
Gavin, I tried jar and a few others I can't recall in Firefox 4.0.1. I've checked wav, mp3 and jnlp in Firefox 5.0 and the behaviour is consistent -- they all default to "Open with" unless "Save File" was used on the last access to the specific filetype.
Whiteboard: [sg:critical] → [sg:critical] [see c#21 for tracking6 status]
blocking1.9.2: .20+ → needed
Gavin: we're running out of time on Firefox 6 -- can we please just increase the delay for 6 and fix it "better" later? You could even hook to the existing "security dialog delay" pref and then people who are really really annoyed can lower it themselves.
Increase the delay to what? I really don't think we want the UX hit of a full-on countdown (a la XPI install), but on the other hand I'm not sure that anything short of that would be a useful mitigation.
(In reply to comment #24)
> but on the other hand I'm not sure
> that anything short of that would be a useful mitigation.

You can focus "Cancel" instead of "OK" if you're looking for a quick fix.
That also significantly impacts usability.

I think the real solution that we want is to avoid offering "open with" from the download confirmation dialog, and instead require that to be done from the download manager later on. That involves adjusting the UI such that there's still a way to configure auto-opening for a given type, unless we want to drop that functionality entirely.
It won't suffice -- watch this video to see what happens if you hold Enter when "Save File" is selected in the confirmation dialog and the "Downloads" box pops up:

http://www.youtube.com/watch?v=_8XpazPu7Yk

You'll have to remove focus from the downloaded file in the "Downloads" box to prevent pressed down Enter from launching the saved file. If you take care of it, you can get away without removing "Open with" by always having "Save File" selected by default (ie. requiring the user to manually select "Open with" each time they want to do it, regardless of the previous choice). I'm not an expert on UX, but removing "Open with" altogether sounds like a huge hit.
(In reply to comment #26)
> I think the real solution that we want is to avoid offering "open with" from
> the download confirmation dialog, and instead require that to be done from
> the download manager later on.

Not offer "open with" ever? Because the problem is that even "safe" types may turn out not to be safe at some future time. I don't see how that's not a bigger usability impact than a simple focus change.
Why is this bug bouncing around a design/ux decision without anyone inviting our UX experts into the discussion?

Faaborg, Limi - we have a problem that the security team considers critical and that Gavin would fix except that we need to know what a fix looks like. Gavin's made some suggestions above, but it would be very helpful to get your insights here, particularly since people in here are agitating for a quick resolution and we want to make sure we do right by our users on security *and* experience.
Ok, here is my quick review of the issue to make sure I understand:

-the user is holding down enter
-firefox starts to download a file type that happens to be executable (and perhaps we didn't know ahead of time that it was executable)
-the save or open with dialog box appears, and Ok has focus instead of Cancel
-user selects ok by holding enter (because it had focus)
-download manager appears, user executes file by holding enter (again it has focus)

So why can't we just fix this by making sure that Cancel has focus in the save or open dialog box instead of Ok? (as an aside, you are never supposed to say Ok, you are supposed to say the action that is going to happen, so in this case it should be download/cancel, or open/cancel depending on what the user selects).
(In reply to comment #30)
> So why can't we just fix this by making sure that Cancel has focus in the
> save or open dialog box instead of Ok?

That would break people who are used to accepting downloads using "Enter". It would also break them in a pretty sucky way, particularly if they have the "don't open download manager" pref set (you'd get the same visual response as before, except the download wouldn't actually start).

Disabling Enter as a way to launch selected files in the Download Manager seems like it would have less of an impact, though it would affect the keyboard accessibility of the download manager.
Not happening for 6. We need to get this moving along though, I'd really hate to ship 7 with this bug as well, and this was filed almost 3 months ago now :(
Faaborg - thoughts on gavin's comment 31?
>though it would affect the keyboard accessibility of the download manager.

I can't seem to find an alternative way for users to launch something out of the download manager with the keyboard (is there a well known command for invoking the context menu on the selected item?)

If there isn't an obvious alternative for launching the file with the keyboard in the download manager that users already know, we are looking at the not incredibly ideal solution of breaking everyone's muscle memory for initiating the download.
blocking1.9.2: needed → .21+
Just to move things along, my recommendation is still that we break muscle memory and move the focus to the cancel button.  Not ideal, but I'm currently unaware of a better solution and we should get this fixed.
Gavin, so it's my understanding the UI team has made a call on a path forward. Are you the right person to get this fixed?
Attached patch remove default button (obsolete) — — Splinter Review
This makes the dialog not have a default button, so by default "Enter" does nothing. You can still tab to the buttons to activate them, though.
Attachment #558976 - Flags: ui-review?(faaborg)
Attachment #558976 - Flags: ui-review?(faaborg) → ui-review+
Comment on attachment 558976 [details] [diff] [review]
remove default button

Unfortunately this is probably going to seem like a bug to many users...
Attachment #558976 - Flags: review?(dolske)
Attachment #558976 - Flags: feedback?(jruderman)
Comment on attachment 558976 [details] [diff] [review]
remove default button

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

Ugh.

I'm marking this r- because it seems like it would be a _major_ UX regression, and I'm not convinced we've looked hard enough at alternatives. [More below.] I'm open to arguments if I've misunderstood the impact/severity, though, but this seems like a high bar. This has likely been an issue since Firefox 1.0 (if not Netscape) days, no?

It might be more expedient for a few of us to just meet and hash out some ideas, but here are a few off-the-cuff proposals:

* Add heuristics so any dialog can make the distinction between "page might be trying to trick user" and the normal case of users just trying to get stuff done. (Strawman: suppress default is there was a previous key/mouse event in last X ms, ignore any new input until quiescent for Y ms).

* Make the default actions safe. EG, Safari seems to prefer just downloading things instead of prompting to open-or-save. Eliminating a prompt is generally UI-win, too.

* Whitelist the kinds of content we'll offer to open (with or without prompting). Goal being to make opening common content (PDF? DOC? ZIP?) as painless as before, but uncommon content intentionally annoying.

* Something like comment 26, but only suppress a default selection on the Downloads window to avoid the problem in comment 27? I'd suspect that there's high usage of the save-or-open dialog, but low(er) usage of opening the default from the download window (eg, Ctrl/Cmd-J + Enter).

Does bug 564934 help/hurt here?

Do other browsers have a clever solution to this that we should adopt?

We also need to sort this out more broadly and consistently than just download dialogs. Seems like addon installs have similar risks (but a different mitigation), and things like WebAPI will add new vectors of concern. And how can we be more formal about where to draw the line (if one must be drawn) -- eg what if a page tries to game a user into closing their eyes and typing a keyboard sequence? Do we need dialog captchas? ;)
Attachment #558976 - Flags: review?(dolske) → review-
The solution here is also probably useful for bug 583175.
So this has been open since May and it's an sg:crit bug. That's way too long. We did have a goal at one point to fix sg:crit bugs in 30days(?). Is there a way we can get to something implementable so we can fix the security issue here?
I confused the status of this bug with those of bug 662309 and bug 663899 (spawned from this bug) with the result that I published this at
http://www.mozilla.org/security/announce/2011/mfsa2011-40.html

Not a detailed account, but it would be best if we could put ANY kind of band-aide in place for Firefox 8.
Whiteboard: [sg:critical] [see c#21 for tracking6 status] → [sg:critical] semi-public :-(
Attached patch don't select an item by default in DM (obsolete) — — Splinter Review
This blocks the Download Manager vector, by not selecting an item by default when it opens.

This leaves open the case where there are file types whose handlers make them effectively "executable", such that we offer to "Open" directly from the dialog. We handled "jar" in separate bugs, and hopefully there aren't any other big ones at the moment. I think we should file a followup to fix that issue.
Attachment #563558 - Flags: review?(dolske)
Comment on attachment 563558 [details] [diff] [review]
don't select an item by default in DM

This seems reasonable.

Per discussion, we can spin off the similar issue with the "Open with / save as" dialog to a new bug, and figure out if we want to similarly wallpaper over the problem, or perhaps just look at jettisoning that dialog in the long term.
Attachment #563558 - Flags: review?(dolske) → review+
https://hg.mozilla.org/mozilla-central/rev/c96e659e0f0a
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
And backed out apparently, don't know why!
https://hg.mozilla.org/mozilla-central/rev/3e43716ba2e8
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch + test fixes — — Splinter Review
Attachment #558976 - Attachment is obsolete: true
Attachment #563558 - Attachment is obsolete: true
Attachment #558976 - Flags: feedback?(jruderman)
https://hg.mozilla.org/mozilla-central/rev/74f57aac5661
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
OS: Windows 7 → All
Hardware: x86 → All
Resolution: --- → FIXED
Attachment #563848 - Flags: approval-mozilla-beta?
Attachment #563848 - Flags: approval-mozilla-aurora?
Comment on attachment 563848 [details] [diff] [review]
patch + test fixes

Approved for Aurora (Update 9) and Beta (Update 8).  Please land as soon as possible.
Attachment #563848 - Flags: approval-mozilla-beta?
Attachment #563848 - Flags: approval-mozilla-beta+
Attachment #563848 - Flags: approval-mozilla-aurora?
Attachment #563848 - Flags: approval-mozilla-aurora+
qa+ for verification with proof of concept in comment 0
Whiteboard: [sg:critical] semi-public :-( → [sg:critical][qa+] semi-public :-(
blocking1.9.2: .23+ → .24+
Comment on attachment 563848 [details] [diff] [review]
patch + test fixes

This applies cleanly on 1.9.2.
Attachment #563848 - Flags: approval1.9.2.24?
Comment on attachment 563848 [details] [diff] [review]
patch + test fixes

approved for 1.9.2.24. Please land asap.
Attachment #563848 - Flags: approval1.9.2.24? → approval1.9.2.24+
Can we land the patch this morning? We're looking to go to build as soon as possible. Thanks!
On 1.9.24, I'm still seeing the (non-exploitable) behavior that I saw in comment 8 many versions ago.

What change was made to 1.9.2.24 compared to that earlier version? The PoC doesn't work pre or post checkin in comment 56 on Windows XP.
(In reply to Al Billings [:abillings] from comment #57)
> On 1.9.24, I'm still seeing the (non-exploitable) behavior that I saw in
> comment 8 many versions ago.
> 
> What change was made to 1.9.2.24 compared to that earlier version? The PoC
> doesn't work pre or post checkin in comment 56 on Windows XP.

The PoC just doesn't cover the case that was fixed. Here's a testcase that does:

1) Load http://gavinsharp.com/os/slow.php?url=/tmp/octet.php&time=5
2) Hold enter while the page loads (should take 5 seconds)

Before fix: enter activates "save as", which opens the download manager, and then activates "open" on the downloaded file (your email application will probably open multiple times)

After fix: enter activtes "save as", which opens the download manager, and then nothing happens after that (since no entry is selected).
Thanks. I've verified this fixed for 1.9.2.24 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.24) Gecko/20111103 Firefox/3.6.24 (.NET CLR 3.5.30729)).
Keywords: verified1.9.2
Any CVE for this one?
Depends on: 704622
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #58)
> (In reply to Al Billings [:abillings] from comment #57)
> > On 1.9.24, I'm still seeing the (non-exploitable) behavior that I saw in
> > comment 8 many versions ago.
> > 
> > What change was made to 1.9.2.24 compared to that earlier version? The PoC
> > doesn't work pre or post checkin in comment 56 on Windows XP.
> 
> The PoC just doesn't cover the case that was fixed. Here's a testcase that
> does:
> 
> 1) Load http://gavinsharp.com/os/slow.php?url=/tmp/octet.php&time=5
> 2) Hold enter while the page loads (should take 5 seconds)
> 
> Before fix: enter activates "save as", which opens the download manager, and
> then activates "open" on the downloaded file (your email application will
> probably open multiple times)
> 
> After fix: enter activtes "save as", which opens the download manager, and
> then nothing happens after that (since no entry is selected).

On Firefox 8, 9, and 10, I still wind up with Notepad opened with your downloaded file from this testcase.

This was tested on Windows XP with a clean profile.

This doesn't seem to be fixed.

Firefox 10: Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111205 Firefox/10.0a2
Reopening based on comment 61.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Please don't reopen FIXED bugs. I'll look into this - we should file a new bug if there ends up being an issue.
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Given comment 63, there is really nothing left for QA to verify on this bug.
Whiteboard: [sg:critical][qa+] semi-public :-( → [sg:critical][qa-] semi-public :-(
(In reply to Al Billings [:abillings] from comment #61)
> > The PoC just doesn't cover the case that was fixed. Here's a testcase that
> > does:
> > 
> > 1) Load http://gavinsharp.com/os/slow.php?url=/tmp/octet.php&time=5
> > 2) Hold enter while the page loads (should take 5 seconds)
> > 
> > Before fix: enter activates "save as", which opens the download manager, and
> > then activates "open" on the downloaded file (your email application will
> > probably open multiple times)
> > 
> > After fix: enter activtes "save as", which opens the download manager, and
> > then nothing happens after that (since no entry is selected).

These were just incorrect STR, sorry about that. Try it with these modifications:

Use http://gavinsharp.com/os/slow.php?url=/tmp/octet.php&time=5 instead.

Before fix: You're prompted to allow the "dangerous executable", or if you've set the pref to never ask, the executable is just launched directly (it's a bogus EXE so it would probably just throw an error)

After fix: not prompted, nothing happens after the download manager opens.
I think part of the problem is that I modified octet.php after your first test, but before your second (it's a test script that I use to test various things). I've put its original contents at message.php (http://gavinsharp.com/os/slow.php?url=/tmp/message.php&time=5) and I won't touch them, you should be able to retry the steps from comment 58 with that URL.
Okay, thanks Gavin. Al, can you please retry with the information given by comment 65 and comment 66?
Whiteboard: [sg:critical][qa-] semi-public :-( → [sg:critical][qa+] semi-public :-(
I ran with http://gavinsharp.com/os/slow.php?url=/tmp/octet.php&time=5 on XP with Firefox 8.0.1 and the Firefox 9, beta 6 build. With both, if I held down the enter key, the mail app wound up launching to open the .eml file.

I expect that it isn't supposed to launch if this is fixed.
That test is not, on Windows, testing what Gavin fixed. .eml is not considered a "dangerous" type and there's a registered handler so you get the Open button by default; you don't fall into the Save path and end up on the download manager list where this fix was.

comment 37 is a fix that resolved the original problem, but it was rejected. What landed and what you're seeing is comment 43. Looks like the follow-up bug was not filed, or at least not linked to this issue.
Daniel, if I can summarize what you are saying...

What Al is seeing (and what I can reproduce) is not addressed by this bug and should be followed up in a new bug.

If this is accurate, how can we mark this bug VERIFIED?
This turns out to be because .eml has a registered handler on Windows.

I tried with .exe files from sourceforge with Firefox 8 and 9b6 and both prompt to save.

So does the .jar case at https://bugzilla.mozilla.org/attachment.cgi?id=541078.
Status: RESOLVED → VERIFIED
Thanks Al, I confirm that too.
Whiteboard: [sg:critical][qa+] semi-public :-( → [sg:critical][qa!] semi-public :-(
To be clear, since I was talking to Dan about this in IRC, if you hold down enter and the download manager comes up, the .exe downloading doesn't have focus and doesn't launch when it is complete. You have to click on or otherwise select the .exe in the download manager to get a prompt, which warns you it is an executable file. This is in both 8.0.1 and 9b6.
Alias: CVE-2011-2372
Group: core-security
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.