1801102 - Assertion failure: offset <= mFd->mLen (Corrupt data offset in JAR file)

Status: RESOLVED FIXED

(Core :: Networking: JAR, defect, P2)

Description

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Using fuzzer in bug 1798631.

#7  0x00007fcec185cb5e in nsZipArchive::GetDataOffset(nsZipItem*) (this=<optimized out>, aItem=0x6190000c35a0) at /mozilla-unified/modules/libjar/nsZipArchive.cpp:808
#8  0x00007fcec18303bb in nsZipArchive::GetData(nsZipItem*) (this=<optimized out>, aItem=0x6190000c35a0) at /mozilla-unified/modules/libjar/nsZipArchive.cpp:820
#9  0x00007fcec185f956 in nsZipCursor::nsZipCursor(nsZipItem*, nsZipArchive*, unsigned char*, unsigned int, bool) (this=0x7ffc444df6e0, item=<optimized out>, aZip=<optimized out>, aBuf=<optimized out>, aBufSize=<optimized out>, doCRC=true) at /mozilla-unified/modules/libjar/nsZipArchive.cpp:1106
#10 0x00007fcec182c104 in nsZipArchive::ExtractFile(nsZipItem*, nsIFile*, PRFileDesc*) (this=0x61d000029e80, item=<optimized out>, outFile=<optimized out>, aFd=<optimized out>) at /mozilla-unified/modules/libjar/nsZipArchive.cpp:464
#11 0x00007fcec182a7a4 in nsZipArchive::Test(char const*) (this=<optimized out>, aEntryName=0x0) at /mozilla-unified/modules/libjar/nsZipArchive.cpp:402
#12 0x00007fcec182a570 in nsJAR::Test(nsTSubstring<char> const&) (this=0x60e000006860, aEntryName=<optimized out>) at /mozilla-unified/modules/libjar/nsJAR.cpp:194
#13 0x00007fcebcd1fd39 in FuzzingRunJARParser(unsigned char const*, unsigned long) (data=<optimized out>, size=0) at /mozilla-unified/netwerk/test/fuzz/TestJARFuzzing.cpp:123
#14 0x00005585e025fbaf in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (this=0x6190000c3080, Data=0x60d000002f60 "\215PK\003\004PK\001\002-T", Size=143) at /mozilla-unified/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:570
#15 0x00005585e0252953 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (F=F@entry=0x6190000c3080, InputFilePath=<optimized out>, MaxLen=0) at /mozilla-unified/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:301
#16 0x00005585e0255d46 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (argc=<optimized out>, argv=<optimized out>, Callback=<optimized out>) at /mozilla-unified/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:810
#17 0x00007fcece318a9d in mozilla::FuzzerRunner::Run(int*, char***) (this=<optimized out>, argc=<optimized out>, argv=<optimized out>) at /mozilla-unified/tools/fuzzing/interface/harness/FuzzerRunner.cpp:75
#18 0x00007fcece20d8f9 in XREMain::XRE_mainStartup(bool*) (this=<optimized out>, aExitFlag=0x7ffc444e1310) at /mozilla-unified/toolkit/xre/nsAppRunner.cpp:4753
#19 0x00007fcece22090b in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=<optimized out>, argc=5, argv=0x7ffc444e29b8, aConfig=<optimized out>) at /mozilla-unified/toolkit/xre/nsAppRunner.cpp:5917
#20 0x00007fcece221526 in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=5, argv=0x7ffc444e29b8, aConfig=...) at /mozilla-unified/toolkit/xre/nsAppRunner.cpp:5985
#21 0x00005585e00d0fee in do_main(int, char**, char**) (argc=5, argv=0x7ffc444e29b8, envp=<optimized out>) at /mozilla-unified/browser/app/nsBrowserApp.cpp:226
#22 main(int, char**, char**) (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at /mozilla-unified/browser/app/nsBrowserApp.cpp:430

The assertion was added in bug 1164141.

Comment 1

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Attachment: testcase.jar

Comment 2

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Attachment: crash-830c8b42b9da1963b540da82991bbf41d4352fc1

May be reproduced by running FUZZER=JARParser obj-asan-opt/dist/bin/firefox crash-830c8b42b9da1963b540da82991bbf41d4352fc1 -use_value_profile=1 -print_pcs=1

Comment 3

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Attachment: Bug 1801102 - Handle corrupted JAR files r=jesup

Comment 4

[Daniel Veditz [:dveditz]]

I don't know if this is practically exploitable, but clearly a malicious person could give us a badly-formed .xpi to trigger this bounds issue. What happens after that if the assertion isn't there?

Comment 5

[BugBot [:suhaib / :marco]]

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:valentin, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Comment 6

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

What happens after that if the assertion isn't there?

Out of bounds read. The attacker doesn't really know where the JAR file will get mapped, or how much they can read until a page fault, but I don't know if that would stop a determined attacker.

Comment 7

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Comment on attachment 9304084 [details]
Bug 1801102 - Handle corrupted JAR files r=jesup

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Easy
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Low risk of regressions. Code returns an error if an invalid offset in the JAR file is encountered or if there's not enough space to read that offset.
• Is Android affected?: Yes

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9304084 [details]
Bug 1801102 - Handle corrupted JAR files r=jesup

Approved to land and request uplift. We disabled jar files from the web, I believe, so that should cut off a lot of the attack surface.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Note that we'll need to uplift the cleanup patch from bug 1164141 to ESR102 as well in order for this to graft cleanly.

Comment 10

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox108 (beta) and tracked for firefox109 (nightly). However, the bug still has low severity.

:ghess, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 11

[Valentin Gosu [:valentin] {{ IETF until 25 July - PTO until 3 August }}]

Comment on attachment 9304084 [details]
Bug 1801102 - Handle corrupted JAR files r=jesup

Beta/Release Uplift Approval Request

• User impact if declined: Out of bounds read when loading specially crafted JAR/xpi file.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk of regressions. Code returns an error if an invalid offset in the JAR file is encountered or if there's not enough space to read that offset.
• String changes made/needed:
• Is Android affected?: Unknown

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high
• User impact if declined: Out of bounds read when loading specially crafted JAR/xpi file.
• Fix Landed on Version: 109
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk of regressions. Code returns an error if an invalid offset in the JAR file is encountered or if there's not enough space to read that offset.

Note that we'll need to uplift the cleanup patch from bug 1164141.

Comment 12

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

Handle corrupted JAR files r=jesup
https://hg.mozilla.org/integration/autoland/rev/5492ba44f59e282dfd383f3837365ddfceb7b2b3
https://hg.mozilla.org/mozilla-central/rev/5492ba44f59e

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9304084 [details]
Bug 1801102 - Handle corrupted JAR files r=jesup

Approved for 108.0b8

Comment 14

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/adf5f8b8fef1

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9304084 [details]
Bug 1801102 - Handle corrupted JAR files r=jesup

Approved for 102.6esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/9968c9743e6a