867056 - (CVE-2013-1700) Arbitrary code execution using a temporarily inaccessible file

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Seb Patane (he/him)]

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:
https://hg.mozilla.org/mozilla-central/file/1eb382609c2d/toolkit/components/maintenanceservice/workmonitor.cpp

Comment 1

[Daniel Veditz [:dveditz]]

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]]

Attachment: Patch v1.

Comment 4

[Brian R. Bondy [:bbondy]]

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 - ex-MoCo]]

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]]

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 7

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e6fb8778820

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2e6fb8778820

Comment 9

[Alex Keybl [:akeybl]]

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]]

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

Comment 11

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9107b23403c5
https://hg.mozilla.org/releases/mozilla-beta/rev/e395208b63be