1336964 - (CVE-2017-7767) Arbitrary file "deletion" as SYSTEM with maintenance service

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Seb Patane (he/him)]

Attachment: proof-of-concept

An unprivileged (standard) user can use the maintenance service to overwrite arbitrary files with junk data. There is no control over what is written to the target file, though the absence of the original data effectively "deletes" it.

For example, running the poc with clobber_file.rb "C:\Program Files (x86)\Mozilla Firefox\updater.exe" will lock firefox to the current installed version until an administrator repairs the installation because the existing updater.exe cannot be loaded to be verified.

The root cause seems to be that if GetTempFileNameW in WriteStatusFailure [0] is called with a path like '\\.\c:\foo\clobbered', instead of returning a temporary file like '\\.\c:\foo\clobbered\svc123.tmp', it just returns the input path. WriteStatusFailure then directly writes to the temporary path [1], truncating it's contents with the results of the updater.

WriteStatusFailure can be called with a user-supplied directory in a few places, the easiest way is to put the target in the first argument and not supply enough further arguments to cause GetInstallationDir in ExecuteServiceCommand to fail [2]. This is how the attached clobber_file.rb proof of concept works.

I haven't tested, but I think the calls in UpdaterIsValid would also be able to be purposely failed with an attacker controlled directory for update.status to get written into. Possibly some of the calls after that point as well, but it gets more difficult once the updater directory is verified as legitimate.

I also tested using hardlinks to redirect the update.status file while using a normal file path to trigger the overwrite - this does not seem to be possible as the maintenance service re-creates the file (which deletes the hardlink, rather than the file that is points to).

Tested on Windows 8.1 x64, Firefox 51.0.1 as well as mozilla-central.

[0] - https://dxr.mozilla.org/mozilla-central/rev/1d025ac534a6333a8170a59a95a8a3673d4028ee/toolkit/mozapps/update/common/updatehelper.cpp#276
[1] - https://dxr.mozilla.org/mozilla-central/rev/1d025ac534a6333a8170a59a95a8a3673d4028ee/toolkit/mozapps/update/common/updatehelper.cpp#288
[2] - https://dxr.mozilla.org/mozilla-central/rev/1d025ac534a6333a8170a59a95a8a3673d4028ee/toolkit/components/maintenanceservice/workmonitor.cpp#601

Comment 1

[Robert Strong (they/them - no direct email)]

Fairly certain that bug 1342742 also fixes this bug.

Matt, I'd like your opinion on whether you agree.

Comment 2

[Molly Howell (she/her) (mostly inactive)]

I agree. The path in question is definitely one of the ones that gets verified now, and these examples would fail that verification.

Comment 3

[Robert Strong (they/them - no direct email)]

Fixed by bug 1342742

[Tracking Requested - why for this release]:
Added since bug 1342742 is tracking for this release and I am planning on landing for this release.

Comment 4

[Robert Strong (they/them - no direct email)]

Bug 1342742 which fixes this bug has been pushed to mozilla-beta and to mozilla-esr52 so setting flags.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Rob, is this a distinct issue from bug 1342742 or is the fix for that other bug broad enough that it covered this as well? (Asking for bounty purposes...)

Comment 6

[Robert Strong (they/them - no direct email)]

This bug is a distinct issue from bug 1342742 and the fix I went with in bug 1342742 was general enough to fix this bug as well.

Comment 7

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Seb, can you verify that this has been fixed?

Comment 8

[Seb Patane (he/him)]

I'm travelling at the moment but should be able to get back to you within the week.

Comment 9

[Seb Patane (he/him)]

I tested using the latest nightly 56.0a1 (2017-07-05) and can confirm the poc no longer replaces file contents.