Closed Bug 1247239 (CVE-2016-5295) Opened 5 years ago Closed 4 years ago
moz maintenance service: Ability to read arbitrary files as SYSTEM
I noticed the problem is known (Bug 850492) but wasn't fixed because the poc was not working. I have developed a poc which relies on the Volume Snapshot Service of windows to copy locked protected files. I tested the poc with the moz maintenance service at C:\Program Files (x86)\Mozilla Maintenance Service and win7 64 bit. If the Volume Snapshot Service is enabled and already a restore point was created (!) the poc is able to copy the C:\Windows\system32\config\SAM file and other protected files.
I'm attempting to fix this by having the service claim an exclusive lock on the copy of updater.exe while performing the copy and the validation steps, so that it's not possible for an outside process to read the file until after we've determined that it's a good updater.exe. Hopefully I'll have a patch ready to post here soon. Rating this as sec-moderate because it can't be exploited remotely without the use of other vulnerabilities to get command execution with arguments.
Assignee: nobody → mhowell
Status: UNCONFIRMED → NEW
Ever confirmed: true
Well, that plan did not survive contact with reality. It turns out most of the validation process involves API's that attempt to open their own handles, so holding one with an exclusive lock keeps those from working, and it isn't exactly worth rewriting them all. There's a second option, which is to still hold that lock while performing the copy, but then apply a tightly limited set of permissions to the copied file and then release the lock. This approach seems to work, in that I can no longer reproduce the bug with that fix in place, and tests are passing locally. So we'll go with that instead, unless a better idea comes along.
Attachment #8726482 - Flags: review?(robert.strong.bugs)
I'm checking out the patch but I have to say that setting restrictive permissions to try to fix these types of issues always scares me since it could possibly lock out part of the process.
Comment on attachment 8726482 [details] [diff] [review] Set tighter permissions on copies of updater.exe created by the maintenance service So, copying the updater was added to prevent an exploit with the maintenance service when we were using the updater from the updates/0 directory. We no longer do that per bug 1127481. It should be possible to not copy the file and run the updater from the install directory for the service as well to fix this.
Attachment #8726482 - Flags: review?(robert.strong.bugs) → review-
Comment on attachment 8737405 [details] [diff] [review] Remove copying of updater.exe for service updates Thanks! At some point we should also remove the following removals for the /update directory as well but I'd like to wait a bit. Could you file a followup for this? http://mxr.mozilla.org/mozilla-central/source/toolkit/components/maintenanceservice/bootstrapinstaller/maintenanceservice_installer.nsi#260 http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/maintenanceservice_installer.nsi#261 http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/maintenanceservice_installer.nsi#321
Attachment #8737405 - Flags: review?(robert.strong.bugs) → review+
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #10) > At some point we should also remove the following removals for the /update > directory as well but I'd like to wait a bit. Could you file a followup for > this? Filed bug 1261978.
This regressed staging updates. It falls back to applying updates without the service. :(
Backed out at Robert's request. https://hg.mozilla.org/integration/mozilla-inbound/rev/b906bf97ccc750b6e9fbf6e8676b303d73be55b1
Merge of backout: https://hg.mozilla.org/mozilla-central/rev/b906bf97ccc7
Okay, here's a different way to deal with this. This patch has the service ignore the updater.exe path parameter and always use the one in the installation directory instead. Also, prior to making that copy, it performs the same verification steps that we run before actually running the updater; otherwise, it would still be possible to use this exploit to read an unrelated file that happens to be named updater.exe.
Comment on attachment 8769296 [details] [diff] [review] Patch - Only copy updater.exe from install path, and verify it first lgtm and thanks!
Attachment #8769296 - Flags: review?(robert.strong.bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cca6d362e38a4eb1f680c62a17137e04b1169f2 Bug 1247239 - Allow the maintenance service to copy updater.exe only from the installation directory, and verify it first; r=rstrong
QA Contact: kjozwiak
How did this land without sec-approval+ on the patch?
I asked for approval on a sec-moderate bug once before and was told I didn't need it. The wiki (https://wiki.mozilla.org/Security/Bug_Approval_Process) confirms that.
Flags: needinfo?(mhowell) → needinfo?(abillings)
Ah, I missed it was sec-moderate. My bad.
Reproduced the issue using the poc from comment#0 with the following build: * https://archive.mozilla.org/pub/firefox/nightly/2016/02/2016-02-10-07-11-15-mozilla-central/ After running the poc from comment#0, the following protected files were copied into the C:\read.poc directory: * C:\Windows\System32\config\SECURITY * C:\Windows\System32\config\SECURITY * C:\Windows\System32\config\software * C:\Windows\System32\config\system Went through verification using the following builds: * Win 7 SP1 x64 - PASSED ** fx52.0a1, buildId: 20161108030212, changeset: f13e90d496cf - PASSED ** fx51.0a2, buildId: 20161108004019, changeset: d9cfe58247e8 - PASSED ** fx50.0b9, buildId: 20161020152750, changeset: 2bb6dc758711 - PASSED
(In reply to Kamil Jozwiak [:kjozwiak] from comment #25) > * C:\Windows\System32\config\SECURITY > * C:\Windows\System32\config\SECURITY > * C:\Windows\System32\config\software > * C:\Windows\System32\config\system The first entry is supposed to be: * C:\Windows\System32\config\SAM
You need to log in before you can comment on or make changes to this bug.