Closed Bug 1234401 (CVE-2017-7796) Opened 9 years ago Closed 8 years ago

Update logger can delete arbitrary files with the name "update.log"

Categories

(Toolkit :: Application Update, defect)

Product:
Toolkit
Component:
Application Update
Platform:
Unspecified
Windows
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr45 --- wontfix
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

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

Details

(Keywords: sec-low, Whiteboard: [adv-main55+][post-critsmash-triage])

Attachments

(2 files, 2 obsolete files)

client patch rev1
2.52 KB, patch
molly
: review+
Details | Diff | Splinter Review
test patch rev1
2.76 KB, patch
molly
: review+
Details | Diff | Splinter Review
On Windows, the update logging system's initialization procedure includes a step that deletes the file that it plans to write to. The name of this file is hard coded as "update.log", but the path to it is read from the command line. The updater can be invoked through the maintenance service, so anyone able to execute that with the right command line can cause any file with the name update.log anywhere on the system to be deleted, using the very high privilege level that the service gets invoked at.
Note: this is a spinoff of bug 1212939 which makes it so only a file named update.log is affected. I am not sure there is a decent way to prevent this from happening at this time.
Attached patch patch in progress (obsolete) — — Splinter Review
Matt, I'm thinking that it should be enough to just require that the path to the patch directory ends with updates\0 for this bug. What do you think?
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Attachment #8860292 - Flags: feedback?(mhowell)
Comment on attachment 8860292 [details] [diff] [review] patch in progress Probably fine. This doesn't accomplish all that much, but I don't think this bug merits any more.
Attachment #8860292 - Flags: feedback?(mhowell) → feedback+
Attachment #8860292 - Attachment is obsolete: true
Attached patch test patch rev1 (obsolete) — — Splinter Review
Try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd4449e37c3fd49e7c3d0b5b211838c3c713ae55 I don't think this is worthwhile uplifting and will likely land this after the other security patches land in early May.
Attached patch test patch rev1 — — Splinter Review
Forgot to update the test description. Comment only change
Attachment #8860567 - Attachment is obsolete: true
Attachment #8860565 - Flags: review?(mhowell)
Attachment #8860572 - Flags: review?(mhowell)
Also pushed to oak so I can manually verify
Attachment #8860565 - Flags: review?(mhowell) → review+
Attachment #8860572 - Flags: review?(mhowell) → review+
Comment on attachment 8860572 [details] [diff] [review] test patch rev1 Pushed to mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/10de1d8666abf1eb61a47545598aeec88222c49e I have patches for beta and esr that I will attach soon.
Merged to mozilla-central https://hg.mozilla.org/mozilla-central/rev/212ac7e82d32 https://hg.mozilla.org/mozilla-central/rev/10de1d8666ab
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Group: toolkit-core-security → core-security-release
Whiteboard: [adv-main55+]
Alias: CVE-2017-7796
Flags: qe-verify-
Whiteboard: [adv-main55+] → [adv-main55+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: