Bug 1336964 (CVE-2017-7767)

Arbitrary file "deletion" as SYSTEM with maintenance service

RESOLVED FIXED in Firefox -esr52

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sebbity, Assigned: rstrong)

Tracking

({csectype-priv-escalation, sec-moderate})

51 Branch
mozilla55
Unspecified
Windows
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox-esr5254+ fixed, firefox53 wontfix, firefox54 fixed, firefox55+ fixed)

Details

(Whiteboard: [adv-main54+][adv-esr52.2+][post-critsmash-triage])

Attachments

(1 attachment)

247 bytes, application/x-ruby
Details
(Reporter)

Description

2 years ago
Posted file 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
(Reporter)

Updated

2 years ago
Summary: Arbitrary file "deletion" as _SYSTEM with maintenance service → Arbitrary file "deletion" as SYSTEM with maintenance service
Flags: sec-bounty?
Fairly certain that bug 1342742 also fixes this bug.

Matt, I'd like your opinion on whether you agree.
Flags: needinfo?(mhowell)
I agree. The path in question is definitely one of the ones that gets verified now, and these examples would fail that verification.
Flags: needinfo?(mhowell)
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Group: toolkit-core-security → core-security-release
Bug 1342742 which fixes this bug has been pushed to mozilla-beta and to mozilla-esr52 so setting flags.
Target Milestone: --- → mozilla55
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...)
Flags: needinfo?(robert.strong.bugs)
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.
Flags: needinfo?(robert.strong.bugs)
Flags: sec-bounty? → sec-bounty+
Hi Seb, can you verify that this has been fixed?
Flags: needinfo?(sebbity)
Whiteboard: [post-critsmash-triage]
(Reporter)

Comment 8

2 years ago
I'm travelling at the moment but should be able to get back to you within the week.
Whiteboard: [post-critsmash-triage] → [adv-main54+][adv-esr52.2+][post-critsmash-triage]
Alias: CVE-2017-7767
(Reporter)

Comment 9

2 years ago
I tested using the latest nightly 56.0a1 (2017-07-05) and can confirm the poc no longer replaces file contents.
Flags: needinfo?(sebbity)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.