Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 867056 - (CVE-2013-1700) Arbitrary code execution using a temporarily inaccessible file
: Arbitrary code execution using a temporarily inaccessible file
: csectype-priv-escalation, sec-high
Product: Toolkit
Classification: Components
Component: Application Update (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla24
Assigned To: Brian R. Bondy [:bbondy]
: Robert Strong [:rstrong] (use needinfo to contact me)
Depends on: CVE-2013-1672
  Show dependency treegraph
Reported: 2013-04-30 00:56 PDT by Seb Patane
Modified: 2014-07-24 16:08 PDT (History)
11 users (show)
abillings: sec‑bounty+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1. (6.63 KB, patch)
2013-05-22 08:24 PDT, Brian R. Bondy [:bbondy]
robert.strong.bugs: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
abillings: sec‑approval+
Details | Diff | Splinter Review

Description Seb Patane 2013-04-30 00:56:38 PDT
In fixing Bug 850492, the updater.exe that the Maintenance Service tries to verify is now copied to a known safe location to stop the user pointing it to a malicious path and replacing the executable after verification.

Unfortunately, if the system cannot copy the file it doesn't error and exit, just logs a warning, uses the malicious path and continues on like normal. 

This introduces a second race condition in that if you have an updater.exe which doesn't exist when execution hits workmonitor.cpp:589 the file copy will fail and the system will use the malicious path. The attacker can then create malicious_path\updater.exe before the program sets the write lock at workmonitor.cpp:332 and then use the same process described in Bug 850492 to execute arbitrary code as SYSTEM.

For convenience, source revision the line numbers are for is latest at the current time and located at:
Comment 1 Daniel Veditz [:dveditz] 2013-05-01 10:39:38 PDT
That seems like a tough timing condition to pull off, but I guess if you're trying to get elevated permissions on your own locked-down machine there's nothing to prevent you from trying over and over until you get it.
Comment 3 Brian R. Bondy [:bbondy] 2013-05-22 08:24:44 PDT
Created attachment 752767 [details] [diff] [review]
Patch v1.
Comment 4 Brian R. Bondy [:bbondy] 2013-05-22 08:28:55 PDT
Comment on attachment 752767 [details] [diff] [review]
Patch v1.

Request is for landing on try, oak (for testing nightly updates), and m-i.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Hard to hit the timing condition but can be done repeatedly until exploited.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Somewhat but it is not completely obvious why the changes are being made.

Which older supported branches are affected by this flaw?
Firefox 21 - 23.

If not all supported branches, which bug introduced the flaw?
bug 850492 tried to fix it where it was reported originally but didn't catch all cases.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Code for other branches should be the same.

How likely is this patch to cause regressions; how much testing does it need?
should land on m-c first since it's an updater task. It is not likely it will cause regressions.
Comment 5 Al Billings [:abillings] 2013-05-22 13:14:04 PDT
Comment on attachment 752767 [details] [diff] [review]
Patch v1.

sec-approval+ for m-c. Please prepare and nominate branch patches for general approval after this is in. 

We might want QA to do some testing on trunk before taking it on branches.
Comment 6 Brian R. Bondy [:bbondy] 2013-05-23 10:26:24 PDT
Tested default settings update in:
- win 8
- win vista admin account
- win vista limited user account installed into app data
- win xp admin account
- win vista limited user account installed into app data

Also tested manually failing the CopyFile to be sure it falls back.
Comment 8 Phil Ringnalda (:philor) 2013-05-23 21:12:01 PDT
Comment 9 Alex Keybl [:akeybl] 2013-05-31 13:46:21 PDT
Please nominate for uplift no later than Monday so that this can make it into our fourth FF22 beta.
Comment 10 Brian R. Bondy [:bbondy] 2013-05-31 18:44:07 PDT
Comment on attachment 752767 [details] [diff] [review]
Patch v1.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 850492
User impact if declined: The fix found in bug 850492 didn't fully address the concern about arbitrary code being executed by the service under elevated privs.
Testing completed (on m-c, etc.): This has been on m-c for a while and updating nightly.
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: none

Note You need to log in before you can comment on or make changes to this bug.