1849518 - crash at null [@ nsZipHandle::findDataStart]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith] (Back Aug 12)]

Attachment: testcase.jar

Found while fuzzing m-c 20230819-981402028450 (--enable-address-sanitizer --enable-fuzzing)

Fuzzing interface docs can be found here: https://firefox-source-docs.mozilla.org/tools/fuzzing/fuzzing_interface.html

FUZZER=JARParser firefox testcase.jar

==11668==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f08b79a083d bp 0x7ffca8825f00 sp 0x7ffca8825d00 T0)
==11668==The signal is caused by a READ memory access.
==11668==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7f08b79a083d in nsZipHandle::findDataStart() /builds/worker/checkouts/gecko/modules/libjar/nsZipArchive.cpp:298:3
    #1 0x7f08b79691ad in nsZipHandle::Init(nsZipArchive*, char const*, nsZipHandle**) /builds/worker/checkouts/gecko/modules/libjar/nsZipArchive.cpp:254:25
    #2 0x7f08b7968712 in nsJAR::OpenInner(nsIZipReader*, nsTSubstring<char> const&) /builds/worker/checkouts/gecko/modules/libjar/nsJAR.cpp:138:10
    #3 0x7f08b2081c52 in FuzzReader(char**, unsigned long*, nsIZipReader*) /builds/worker/checkouts/gecko/netwerk/test/fuzz/TestJARFuzzing.cpp:149:23
    #4 0x7f08b2083090 in FuzzingRunJARParser(unsigned char const*, unsigned long) /builds/worker/checkouts/gecko/netwerk/test/fuzz/TestJARFuzzing.cpp:180:10
    #5 0x56022e8ebc6b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:570:11
    #6 0x56022e8eb6f1 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:479:7
    #7 0x56022e8ecb27 in fuzzer::Fuzzer::MutateAndTestOne() /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:717:19
    #8 0x56022e8ed535 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:861:9
    #9 0x56022e8ddeeb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:864:14
    #10 0x7f08c69a98ab in mozilla::FuzzerRunner::Run(int*, char***) /builds/worker/checkouts/gecko/tools/fuzzing/interface/harness/FuzzerRunner.cpp:75:13
    #11 0x7f08c68c0cd4 in XREMain::XRE_mainStartup(bool*) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:4661:35
    #12 0x7f08c68d1d4c in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5862:12
    #13 0x7f08c68d3031 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5930:21
    #14 0x56022e718ee3 in do_main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:227:22
    #15 0x56022e718ee3 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:445:16
    #16 0x7f08dd135082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
    #17 0x56022e6428d8 in _start (/home/worker/firefox/firefox+0x1068d8) (BuildId: 051a86864e4ff8d8e4602f6c8e74b229a25cc0fa)

Comment 1

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

Attachment: Bug 1849518 - Add null after call to FileLocation::GetBaseFile r=jesup

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ace7a44294d5
Add null after call to FileLocation::GetBaseFile r=jesup

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ace7a44294d5

Comment 4

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:valentin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 5

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

Comment on attachment 9351756 [details]
Bug 1849518 - Add null after call to FileLocation::GetBaseFile r=jesup

Beta/Release Uplift Approval Request

• User impact if declined: Potential crash with corrupt jar file
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• 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): Just a null check before dereferencing.
• String changes made/needed:
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Low risk fix for a rare crash.
• User impact if declined: Potential crash with corrupt jar file
• Fix Landed on Version: 119
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just a null check before dereferencing.

Comment 6

[Pascal Chevrel (relman team) -> :pascalc]

Comment on attachment 9351756 [details]
Bug 1849518 - Add null after call to FileLocation::GetBaseFile r=jesup

Approved for 118.0b8, thanks.

Comment 7

[Pulsebot]

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

Comment 8

[Pascal Chevrel (relman team) -> :pascalc]

Comment on attachment 9351756 [details]
Bug 1849518 - Add null after call to FileLocation::GetBaseFile r=jesup

Approved for ESR 115.3, thanks.

Comment 9

[Pascal Chevrel (relman team) -> :pascalc]

https://hg.mozilla.org/releases/mozilla-esr115/rev/8bb39dee1f5b

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/8bb39dee1f5b