Closed Bug 1246944 (CVE-2016-5253) Opened 8 years ago Closed 8 years ago

Arbitrary file overwrite with updater and moz maintenance service using callback application path parameter

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox48 --- verified

People

(Reporter: hofusec, Assigned: molly)

Details

(Keywords: sec-moderate, Whiteboard: [adv-main48+])

Attachments

(2 files, 1 obsolete file)

Attached file ovr.poc.zip
During an update using the callback application path parameter a copy of an user specified file (the callback file) is made. For example for <callback-file> a <callback-file>.moz-callback is created. If the target of the copy is prepared with a locked hardlink an arbitrary file is replaced.

The poc use the CreateHardlink tool from https://github.com/google/symboliclink-testing-tools to create the hardlink without the need of write access to the target file. 
I have tested the poc with win7 and ff 47 nightly installed to C:\Program Files (x86)\Nightly\:
To reproduce
- download poc and extract the files
- download http://releases.mozilla.org/pub/firefox/nightly/latest-mozilla-central/firefox-47.0a1.en-US.win32.complete.mar and place the file to <pocdir>/updatework/update.mar
- execute poc.rb
The poc replace the content of the firefox.exe with the content of the cmd.exe. 

This can be easily used to elevate privileges.
Rob, can you take this and suggest a security rating?
Flags: needinfo?(robert.strong.bugs)
This bug allows overwriting any file, under SYSTEM privileges, given that the attacker can run a command with arbitrary arguments. Since Windows doesn't have anything like setuid, this does not mean that code can be made to execute at a higher privilege level than it normally would, but it does mean that files normally executed with high privileges can be replaced with malicious code.

The quick way that I see to fix this would be to delete any file that already exists at the callback file copy location prior to creating the copy. In terms of this exploit, that would break the link and the new copy we create would actually be a new file. But in the more general case, deleting a file always has to be done with caution. I think it seems valid here because we could safely assume that some previous (presumably failed) update attempt created anything that already exists at that path, so we should be clear to delete it. But I still don't feel 100% great about that solution, so I'll poke at it a little more before I submit that patch.
Assignee: nobody → mhowell
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(robert.strong.bugs)
Keywords: sec-moderate
Attachment #8729578 - Flags: review?(robert.strong.bugs)
Comment on attachment 8729578 [details] [diff] [review]
Don't continue updating if copying the callback executable fails



>diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp
>--- a/toolkit/mozapps/update/updater/updater.cpp
>+++ b/toolkit/mozapps/update/updater/updater.cpp
>@@ -3214,17 +3214,34 @@ int NS_main(int argc, NS_tchar **argv)
>       *d = NS_T('\0');
>       ++d;
> 
>       // Make a copy of the callback executable so it can be read when patching.
>       NS_tsnprintf(gCallbackBackupPath,
>                    sizeof(gCallbackBackupPath)/sizeof(gCallbackBackupPath[0]),
>                    NS_T("%s" CALLBACK_BACKUP_EXT), argv[callbackIndex]);
>       NS_tremove(gCallbackBackupPath);
>-      CopyFileW(argv[callbackIndex], gCallbackBackupPath, false);
>+      if(!CopyFileW(argv[callbackIndex], gCallbackBackupPath, TRUE)) {
Please change TRUE to lowercase for consistency with the rest of the file since we switched to lowercase a couple of years ago.

>+        DWORD copyFileError = GetLastError();
>+        LOG(("NS_main: failed to copy callback file " LOG_S
>+             " into place at " LOG_S, argv[callbackIndex], gCallbackBackupPath));
>+        LogFinish();
>+        if (copyFileError == ERROR_ACCESS_DENIED) {
>+          WriteStatusFile(WRITE_ERROR_ACCESS_DENIED);
>+        } else {
>+          WriteStatusFile(WRITE_ERROR_CALLBACK_APP);
>+        }
>+
>+        EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
>+        LaunchCallbackApp(argv[callbackIndex],
>+                          argc - callbackIndex,
>+                          argv + callbackIndex,
>+                          sUsingService);
>+        return 1;
>+      }
> 
>       // Since the process may be signaled as exited by WaitForSingleObject before
>       // the release of the executable image try to lock the main executable file
>       // multiple times before giving up.  If we end up giving up, we won't
>       // fail the update.
>       const int max_retries = 10;
>       int retries = 1;
>       DWORD lastWriteError = 0;
Attachment #8729578 - Flags: review?(robert.strong.bugs) → review+
Patch including review change and rebased.
Attachment #8729578 - Attachment is obsolete: true
Attachment #8733992 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/3eb4d6eee3e4
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Group: toolkit-core-security → core-security-release
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Reproduced the original issue using the STR/POC from comment #0 using the following builds:
* build: https://archive.mozilla.org/pub/firefox/nightly/2016/03/2016-03-01-03-02-37-mozilla-central/firefox-47.0a1.en-US.win32.installer.exe
* mar: https://archive.mozilla.org/pub/firefox/nightly/2016/03/2016-03-03-03-02-53-mozilla-central/firefox-47.0a1.en-US.win32.complete.mar

* Win 7 x64 VM - Reproduced
* Win 10 x64 VM - Reproduced

Used the following builds for verification:

* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-20-03-02-08-mozilla-central/firefox-50.0a1.en-US.win32.installer.exe
* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-22-03-02-35-mozilla-central/firefox-50.0a1.en-US.win32.complete.mar
* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-19-00-40-28-mozilla-aurora/firefox-49.0a2.en-US.win32.installer.exe
* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-22-00-40-32-mozilla-aurora/firefox-49.0a2.en-US.win32.complete.mar
* https://archive.mozilla.org/pub/firefox/candidates/48.0b7-candidates/build1/win32/en-US/Firefox Setup 48.0b7.exe
* https://archive.mozilla.org/pub/firefox/candidates/48.0b9-candidates/build1/update/win32/en-US/firefox-48.0b9.complete.mar

OS's Used:

* Win 10 x64 VM & Win 7 x64 VM - PASSED
** fx50.0a1 buildId: 20160720030208, changeset: ed8e23b5e0c7 - PASSED
** fx49.0a2 buildId: 20160719004028, changeset: c1cee3d09904 - PASSED
** fx48.0b7 buildId: 20160711002726, changeset: 9d734024ed35 - PASSED
Status: RESOLVED → VERIFIED
Alias: CVE-2016-5253
Whiteboard: [adv-main48+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.