Closed Bug 1336964 (CVE-2017-7767) Opened 7 years ago Closed 7 years ago

Arbitrary file "deletion" as SYSTEM with maintenance service

Categories

(Toolkit :: Application Update, defect)

51 Branch
Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 54+ fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 + fixed

People

(Reporter: sebbity, Assigned: robert.strong.bugs)

References

Details

(Keywords: csectype-priv-escalation, sec-moderate, Whiteboard: [adv-main54+][adv-esr52.2+][post-critsmash-triage])

Attachments

(1 file)

Attached 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
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
Closed: 7 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]
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
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.

Attachment

General

Created:
Updated:
Size: