925747 - (CVE-2014-1496) Files extracted from Mar file are not locked during update

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Holger Fuhrmannek]

Files like update.manifest or *.patch files arent locked and extracted in a writeable place during an update process so it is possible to change the update process. With manipulted *.patch and update.manifest files it seems to be possible to change any files which are relative to the working directory of the updater (for example the firefox directory).
I have tested this behavoir in ff 24.0.

Comment 1

[Matt Wobensmith [:mwobensmith][:matt:]]

A PoC would be really helpful.

Also, in rating this bug, we are trying to understand if the attack is mitigated enough to call it a sec-moderate, or if it can be used on its own for something worse.

Comment 2

[Holger Fuhrmannek]

I have uploaded a poc, you need ruby to run the poc. 
The poc is for firefox 24.0, tested with win7:

To run the poc do the following steps:

- create a poc directory
- copy the updater.exe from the firefox directory to the poc directory
- download http://download.mozilla.org/?product=firefox-24.0-partial-23.0 and save the file as update.mar in the poc directory
- download poc.zip and extract the files
- alter the paths in start.bat and exp.rb to your paths
- start start.bat and exp.rb
- wait until the xul.dll in the firefox directory has a size of <1kb
- open xul.dll -> the content will be "ABCDEFGH"

Comment 3

[Holger Fuhrmannek]

Attachment: poc.zip

Comment 4

[Matt Wobensmith [:mwobensmith][:matt:]]

I'm running this, but I don't see the issue yet. The xul.dll file is not modified from its starting size of around 22mb. 

Is there an order to starting those two files? I started the ruby file first and then the bat file, but I don't see any change.

How long do your scripts run until you notice the change to xul.dll? I ran it for 20 minutes without seeing any modification.

Comment 5

[Holger Fuhrmannek]

That's bad, it's working for me after a few minutes but the exploit needs the right timing.
There is no order to start the files.
Can you give me the content of your update.log in the poc directory?

Comment 6

[Matt Wobensmith [:mwobensmith][:matt:]]

I assume you meant the contents of update.manifest, right?

type "partial"
patch "xul.dll.patch" "xul.dll"

Comment 7

[Matt Wobensmith [:mwobensmith][:matt:]]

When running this, I see a folder called "tobedeleted" added to the Firefox folder, which eventually disappears between iterations of the script(s). Dunno if that's just the updater in action or if it signifies that something bad is happening.

I also saw a file appear and then disappear called "xul.dll_mozbackup" or something similar. Can't get it to happen regularly. However, the xul.dll always maintains the same file size and creation/modification dates.

Comment 8

[Holger Fuhrmannek]

I actually mean the update.log, located in the directory of the update.mar.
It seems the update is running but because of a failure, the update isn't completed.
The source of the failure is interesting, that is the reason why I need the update.log.

Comment 9

[Matt Wobensmith [:mwobensmith][:matt:]]

No update.log file is created here. That might be telling us something.

Comment 10

[Matt Wobensmith [:mwobensmith][:matt:]]

OK, I see a problem on my end. The path to the exploit directory in my BAT file had a trailing slash, but in your example, it does not. Trying this again...

Comment 11

[Matt Wobensmith [:mwobensmith][:matt:]]

Indeed, the extra trailing slash was the problem. Once I corrected it, I was able to reproduce the problem within 10 minutes. 

So, I can confirm. Thanks for your patience.

Comment 12

[chris hofmann]

rstrong, can you have a look at this and help get it assigned?   we also need to figure out severity for the bounty program.

Comment 13

[Robert Strong (they/them - no direct email)]

I'll take this.

This has been the around since this updater was implemented for Firefox 1.5. It requires access to the filesystem and per comment #5 is timing dependent.

Comment 14

[Robert Strong (they/them - no direct email)]

Also, I'll try to get to this sometime in the next several weeks.

Comment 15

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Brian, we already extract new / complete files to the destination location and name and think for patch files they should just be created in the destination directory instead. I also did the same for the manifest. Seem like a plan?

Comment 16

[Robert Strong (they/them - no direct email)]

BTW: this only affects patching existing files. Files that are added (complete update or new file during a partial) are extracted to the installation directory. The patch essentially does the same when patching files.

Comment 17

[Robert Strong (they/them - no direct email)]

Daniel, how can this be used to "local priv escalation to admin"? As I understand it this bug allows changing the files in the install dir but nothing is launched elevated.

Comment 18

[Brian R. Bondy [:bbondy]]

Comment on attachment 8360950 [details] [diff] [review]
patch rev1

Review of attachment 8360950 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds good to me but if you think it is less work we could also just CreateFile with a 0 for the sharing flags.
Your way works because if they installed to a low integrity location they could just modify those files in the first place.

Comment 19

[Robert Strong (they/them - no direct email)]

I think this is much simpler / safer and works cross platform when we need it.

Comment 20

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

There is the possibility however unlikely that patch files or the manifest might be left behind in which case the code signing check on Mac will fail so I went with using a directory instead. I also excluded Updated.app in CodeResources since it was overlooked.

I'll update the tests to check for this directory in a separate patch

Comment 21

[Brian R. Bondy [:bbondy]]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

Review of attachment 8361291 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/update/updater/updater.cpp
@@ +1399,4 @@
>  
>    NS_tremove(spath);
>  
>    FILE *fp = NS_tfopen(spath, NS_T("wb"));

I'm pretty sure this will fail because the updating directory doesn't exist.  Let me know if it is being created somewhere that I don't see though.

Comment 22

[Robert Strong (they/them - no direct email)]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2


> int DoUpdate()
> {
>   NS_tchar manifest[MAXPATHLEN];
>   NS_tsnprintf(manifest, sizeof(manifest)/sizeof(manifest[0]),
>-               NS_T("%s/update.manifest"), gSourcePath);
>+               NS_T("%s/updating/update.manifest"), gDestinationPath);
>+  ensure_parent_dir(manifest);
Right here

Comment 23

[Robert Strong (they/them - no direct email)]

BTW: tests pass locally and I will be sending the patch to try soon as well as after I update the tests

Comment 24

[Brian R. Bondy [:bbondy]]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

Review of attachment 8361291 [details] [diff] [review]:
-----------------------------------------------------------------

:)

Comment 25

[Robert Strong (they/them - no direct email)]

try run without updated tests
https://tbpl.mozilla.org/?tree=Try&rev=73ef8f231cee

Comment 26

[Robert Strong (they/them - no direct email)]

Attachment: test patch

Comment 27

[Robert Strong (they/them - no direct email)]

Pushed to try for Win, Mac, and Linux opt and dbg
https://tbpl.mozilla.org/?tree=Try&rev=9f804fd70bce

The B2G emulator debug failures are not related to this... they are in part due to B2G disabling app update tests several months ago and something as of yet unknown changing that has caused the tests to fail... I am investigating. The bug for re-enabling the app update tests is bug 959327.

Pushed to try for B2G emulator opt
https://tbpl.mozilla.org/?tree=Try&rev=5ecaf450f275

Comment 28

[Brian R. Bondy [:bbondy]]

Comment on attachment 8362190 [details] [diff] [review]
test patch

Review of attachment 8362190 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for adding this, I meant to ask for such a test :)

Comment 29

[Robert Strong (they/them - no direct email)]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

Steven, could you review the CodeResources changes? Thanks!

Comment 30

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

This looks fine to me.

It makes sense that both the Updated.app and the updating directories be excluded from what gets signed.  Updated.app has been around for a while, and wasn't excluded before, and this apparently didn't cause trouble.  But on general principles it does make sense to exclude it.

The syntax looks fine -- it matches what's already in the file, and Apple's doc here:

https://developer.apple.com/library/mac/technotes/tn2206/_index.html#//apple_ref/doc/uid/DTS40007919-CH1-SUBSECTION10

Comment 31

[Robert Strong (they/them - no direct email)]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

AIUI this exploit can leave the user with an install of Firefox that has modified files and nothing could be exploited that the original process used to exploit wouldn't already be able to accomplish though I could be wrong. Already needinfo'd dveditz in comment #17.

[Security approval request comment]
How easily could an exploit be constructed based on the patch? 
Not sure. The exploiter would need to have local system access first, we haven't heard of these happening in the wild, and it is unclear whether this would be of value to exploit (see previous comment).

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No

Which older supported branches are affected by this flaw?
All since Firefox 1.5

If not all supported branches, which bug introduced the flaw?
N/A

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Yes

How likely is this patch to cause regressions; how much testing does it need?
Not too likely. I'd like to have it land on m-c for a few days before uplifting to other branches.

Comment 32

[Al Billings [:abillings - ex-MoCo]]

This may be too late in the cycle to take on Beta, which means we may wish to delay taking it at all until a few weeks into the next cycle.

We need Release Management to weigh in here.

Comment 33

[bhavana bajaj [:bajaj]]

(In reply to Al Billings [:abillings] from comment #32)
> This may be too late in the cycle to take on Beta, which means we may wish
> to delay taking it at all until a few weeks into the next cycle.
+1 here given that this would need some bake time on m-c per comment #31 and keeping in mind how close we are in releasing Fx27 with one-two beta's left and this seems to be as old as Firefox 1.5.
> 
> We need Release Management to weigh in here.

Comment 34

[Daniel Veditz [:dveditz]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #17)
> Daniel, how can this be used to "local priv escalation to admin"? As I
> understand it this bug allows changing the files in the install dir but
> nothing is launched elevated.

I misunderstood the scenario a little, but on a multi-account system it would be possible to modify the shared copy of Firefox and then wait for a privileged user to use it. Or as in my case, I normally use a lower privileged account for security reasons but will input my admin password to UAC prompts when given by trusted programs for reasonable-seeming reasons. I personally am less likely to be fooled by a modified Firefox because I know it so well, but I suspect I could be fooled if one of my other programs were similarly modified and thus think that a Firefox hack would work on a fair number of people.

Not very useful as a broad attack, but could be effective as a targeted attack if some other exploit got you a foothold in an unprivileged account. That more limited scope than implied by the current sec-high so I'll lower it a bit.

Comment 35

[Daniel Veditz [:dveditz]]

We should fix this in Firefox 28 though. It won't take too much nightly testing to know whether it's working or not. Interpreting comment 33 as a wontfix for Fx27

Comment 36

[Robert Strong (they/them - no direct email)]

Thanks Daniel!

Just to verify, security is ok with this landing on mozilla-central at this time?

Comment 37

[Holger Fuhrmannek]

I'm certain this can lead to privilege escalation. Maybe with the maintenanceservice_installer.exe because it is executed with system rights from the firefox directory during an update.

Comment 38

[Al Billings [:abillings - ex-MoCo]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #36)
> Thanks Daniel!
> 
> Just to verify, security is ok with this landing on mozilla-central at this
> time?

Only after sec-approval+ is given, which I'm about to do.

Comment 39

[Robert Strong (they/them - no direct email)]

Pushed both patches to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4177c7f6f8b

Comment 40

[Carsten Book [:Tomcat]]

landed this patches on central in https://hg.mozilla.org/mozilla-central/rev/c4177c7f6f8b

Comment 41

[Holger Fuhrmannek]

I have created a second poc which replace the maintenanceservice_installer.exe in the firefox directory with a payload which start a calc.exe with system rights during an update.
In my opionion shows this, that it´s easy to get system privileges with this security vulnerability.

I have tested the poc2 with ff 26.0:

- create a poc directory
- copy the updater.exe from the firefox directory to the poc directory
- download http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/26.0/update/win32/en-US/firefox-25.0-26.0.partial.mar and save the file as update.mar in the poc directory
- download poc2.zip and extract the files in the poc directory
- alter the paths in start.bat and exp.rb to your paths
- start start.bat and exp.rb
- wait until the maintenanceservice_installer.exe in the firefox directory has a size of 73kb
- check for calc.exe with system rights

Comment 42

[Holger Fuhrmannek]

Attachment: poc2.zip

Comment 43

[Holger Fuhrmannek]

So i think it's sec-high vulnerability.

Comment 44

[Robert Strong (they/them - no direct email)]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Initial implementation of app update in Firefox 1.5
User impact if declined: If a system is already compromised locally it is possible to modify files in the install directory and gain admin access.
Testing completed (on m-c, etc.): it has been on m-c for several days and tests pass.
Risk to taking this patch (and alternatives if risky): minor... the patch changes the directory used to extract patches to a location in the install directory.
String or IDL/UUID changes made by this patch: none

Comment 45

[Robert Strong (they/them - no direct email)]

Attachment: aurora test patch

Comment 46

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/a994c03f25d4

Comment 47

[Robert Strong (they/them - no direct email)]

Comment on attachment 8361291 [details] [diff] [review]
patch rev2

Just noticed the status-firefox-esr24: affected so I am assuming this is wanted there as well.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: If a system is already compromised locally it is possible to modify files in the install directory and gain admin access.
Fix Landed on Version: Firefox 28
Risk to taking this patch (and alternatives if risky): minor... the patch changes the directory used to extract patches to a location in the install directory.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 48

[Al Billings [:abillings - ex-MoCo]]

Given that we're about to generate builds for ESR release, this probably can't be approved and go in now. It will need to wait.

Comment 49

[Al Billings [:abillings - ex-MoCo]]

Dan, what do you think of sec-moderate versus sec-high here?

Comment 50

[Daniel Veditz [:dveditz]]

It's kind of arbitrary. For the vast majority of our users this isn't a big deal because if they have local malware they're already done for, and they own their machine so privilege escalation is uninteresting. In a more corporate environment (such as those for whom the ESR is intended) this is a bigger deal, but then again those folks are more likely to have their own configuration tools and might not even allow our maintenance service.

I don't care which we call it, it depends on whose POV you're using. I think Ash deserves a bug bounty for it if that's what's really behind your question.

Comment 52

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr24/rev/7efebaa354ba

Comment 53

[Kamil Jozwiak [:kjozwiak]]

Before going through the verification process, I reproduced the original issue from comment #2 three different times. I couldn't reproduce the second POC that was added in comment #41 so I used the POC from comment #2 for verification:

Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-03-13-03-02-02-mozilla-central/firefox-30.0a1.en-US.win32.installer.exe
mar file: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-03-13-03-02-02-mozilla-central/firefox-30.0a1.en-US.win32.complete.mar
OS: Windows 7 Pro SP1 x64

- Ran through the POC from comment #2 and couldn't reproduce the original issue after running for 15 minutes. The xul.dll file was constantly being replaced by the backup while the POC was running

Build: 	http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-03-13-00-40-00-mozilla-aurora/firefox-29.0a2.en-US.win32.installer.exe
mar file: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-03-13-00-40-00-mozilla-aurora/firefox-29.0a2.en-US.win32.complete.mar
OS: Windows 7 Pro SP1 x64

- Ran through the POC from comment #2 and couldn't reproduce the original issue after running for 15 minutes. The xul.dll file was constantly being replaced by the backup while the POC was running

Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/28.0b9/win32/en-US/Firefox Setup 28.0b9.exe
mar file: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/28.0b9/update/win32/en-US/firefox-28.0b9.complete.mar
OS: Windows 7 Pro SP1 x64

- Ran through the POC from comment #2 and couldn't reproduce the original issue after running for 15 minutes. The xul.dll file was constantly being replaced by the backup while the POC was running

Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-esr24/firefox-24.3.0esrpre.en-US.win32.installer.exe
mar file: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/28.0b9/update/win32/en-US/firefox-24.3.0esrpre.en-US.win32.complete.mar
OS: Windows 7 Pro SP1 x64

- Ran through the POC from comment #2 and couldn't reproduce the original issue after running the POC for 15 minutes.

Marking this issue as verified. Please re-open if there's any issues or something was missed in these test cases.