1811939 - (CVE-2023-25739) Heap-use-after-free in mozilla::dom::ScriptLoadContext::~ScriptLoadContext

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Holger Fuhrmannek]

Attachment: asan_log.txt

After a few seconds the following testcase triggers a heap-use-after-free
with a current asan build of firefox (product_version = 20230123-52f9e6382dce, installed via fuzzfetch --central -a).

<script>
    import("data:text/javascript,0").catch(x => 0);
    import("data:text/javascript,0").catch(x => 0);
    window.stop();
</script>

==2398430==ERROR: AddressSanitizer: heap-use-after-free on address 0x61400000b640 at pc 0x7facd4b345e3 bp 0x7ffd70bfa260 sp 0x7ffd70bfa258
READ of size 8 at 0x61400000b640 thread T0 (file:// Content)
    #0 0x7facd4b345e2 in Release /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:50:40
    #1 0x7facd4b345e2 in Release /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:381:36
    #2 0x7facd4b345e2 in assign_assuming_AddRef /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:69:7
    #3 0x7facd4b345e2 in operator= /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:168:5
    #4 0x7facd4b345e2 in mozilla::dom::ScriptLoadContext::~ScriptLoadContext() /builds/worker/checkouts/gecko/dom/script/ScriptLoadContext.cpp:77:12
    #5 0x7facd4b346ad in mozilla::dom::ScriptLoadContext::~ScriptLoadContext() /builds/worker/checkouts/gecko/dom/script/ScriptLoadContext.cpp:71:41
...

The testcase seems similar to the testcases of Bug 1291535 and Bug 1529203, maybe the issues are related.

Comment 1

[Holger Fuhrmannek]

Attachment: testcase.html

Comment 2

[Andrew McCreight [:mccr8]]

Thanks for the report. We've been seeing a UAF like this in our crash reports but we haven't been able to figure out what is causing it.

Jon, this stack looks like bug 1805019, so maybe here's a test case to make it actionable.

Comment 3

[Jon Coppeard (:jonco)]

I can reproduce this locally. In debug builds it hits an assertion:

#6  0x00007ffff7a85520 in <signal handler called> () at /lib/x86_64-linux-gnu/libc.so.6
#7  0x00007fffe405a599 in mozilla::LinkedList<JS::loader::ScriptLoadRequest>::assertContains(JS::loader::ScriptLoadRequest*) const (this=0x7fffc1d76608, aValue=0x7fffc0f9e100)
    at /home/jon/clone/modules/default-build/dist/include/mozilla/LinkedList.h:696
#8  0x00007fffe405a4c1 in mozilla::LinkedListElement<JS::loader::ScriptLoadRequest>::removeFrom(mozilla::LinkedList<JS::loader::ScriptLoadRequest> const&)
    (this=0x7fffc0f9e108, aList=const mozilla::LinkedList<JS::loader::ScriptLoadRequest> &) at /home/jon/clone/modules/default-build/dist/include/mozilla/LinkedList.h:281
#9  0x00007fffe4051081 in JS::loader::ScriptLoadRequestList::Remove(JS::loader::ScriptLoadRequest*) (this=0x7fffc1d76608, aElem=0x7fffc0f9e100)
    at /home/jon/clone/modules/default-build/dist/include/js/loader/ScriptLoadRequest.h:402
#10 0x00007fffe403fff4 in JS::loader::ModuleLoaderBase::RemoveDynamicImport(JS::loader::ModuleLoadRequest*) (this=0x7fffc1d765b0, aRequest=0x7fffc0f9e100)
    at /home/jon/clone/modules/js/loader/ModuleLoaderBase.cpp:1017
#11 0x00007fffe403fcca in JS::loader::ModuleLoadRequest::LoadFinished() (this=0x7fffc0f9e100) at /home/jon/clone/modules/js/loader/ModuleLoadRequest.cpp:187
#12 0x00007fffe403ff5a in JS::loader::ModuleLoadRequest::LoadFailed() (this=0x7fffc0f9e100) at /home/jon/clone/modules/js/loader/ModuleLoadRequest.cpp:181
#13 0x00007fffe4060beb in mozilla::MozPromise<bool, nsresult, false>::InvokeMethod<JS::loader::ModuleLoadRequest, void (JS::loader::ModuleLoadRequest::*)(), nsresult const&>(JS::loader::ModuleLoadRequest*, void (JS::loader::ModuleLoadRequest::*)(), nsresult const&)
    (aThisVal=0x7fffc0f9e100, aMethod=(void (JS::loader::ModuleLoadRequest::*)(class JS::loader::ModuleLoadRequest * const)) 0x7fffe403fe60 <JS::loader::ModuleLoadRequest::LoadFailed()>, aValue=@0x7fffc0ffd5e0: nsresult::NS_BINDING_ABORTED) at /home/jon/clone/modules/default-build/dist/include/mozilla/MozPromise.h:639

I'm not sure why this leads to the use-after-free in ~ScriptLoadContext.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1811939 - Check whether module load request was already cancelled when a load fails r?smaug

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1811939 - Add testcase r?smaug

Depends on D167927

Comment 6

[Jon Coppeard (:jonco)]

Comment on attachment 9314226 [details]
Bug 1811939 - Check whether module load request was already cancelled when a load fails r?smaug

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not particularly easily.
• 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, code is unchanged since 2017
• 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?: Should be trivial.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, it's a simple patch which adds a defensive check against repeated cancellation.
• Is Android affected?: Yes

Comment 7

[Tom Ritter [:tjr]]

Comment on attachment 9314226 [details]
Bug 1811939 - Check whether module load request was already cancelled when a load fails r?smaug

Approved to land and request uplift

Comment 8

[Tom Ritter [:tjr]]

We can land the test at the end of March

Comment 9

[Julien Cristau [:jcristau]]

Check whether module load request was already cancelled when a load fails r=smaug
https://hg.mozilla.org/integration/autoland/rev/2a4428926a49
https://hg.mozilla.org/mozilla-central/rev/2a4428926a49

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

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

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

For more information, please visit auto_nag documentation.

Comment 11

[Jon Coppeard (:jonco)]

Comment on attachment 9314226 [details]
Bug 1811939 - Check whether module load request was already cancelled when a load fails r?smaug

Beta/Release Uplift Approval Request

• User impact if declined: Possible crash / security vulnerability.
• 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): This is a simple change that is covered by tests.
• 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: This is a set-high bug.
• User impact if declined: Possible crash / security vulnerability.
• Fix Landed on Version: 111
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a simple change that is covered by tests.

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9314226 [details]
Bug 1811939 - Check whether module load request was already cancelled when a load fails r?smaug

Approved for 110 beta 8, thanks.

Comment 13

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/3521dd2cbc3f

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9314226 [details]
Bug 1811939 - Check whether module load request was already cancelled when a load fails r?smaug

Approved for ESR 102.8.0, thanks.

Comment 15

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-esr102/rev/7a32cb8720f6

Comment 16

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

Copying crash signatures from duplicate bugs.

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

a month ago, Tom Ritter [:tjr] placed a reminder on the bug using the whiteboard tag [reminder-test 2023-03-28] .

jonco, please refer to the original comment to better understand the reason for the reminder.

Comment 21

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

Add testcase r=smaug
https://hg.mozilla.org/integration/autoland/rev/f1cf8fd3ff6d436ee62346ea26b3d071aaad7170
https://hg.mozilla.org/mozilla-central/rev/f1cf8fd3ff6d