1728574 - Crash [@ mozilla::dom::RTCStatsTimestampMaker::RTCStatsTimestampMaker]

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev f4ad9b76e5f8 (built with: --enable-address-sanitizer --enable-fuzzing). Filing this against WebRTC though I think it is likely due to the call to window.print().

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build f4ad9b76e5f8 --asan --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.zip --repeat 10

[@ mozilla::dom::RTCStatsTimestampMaker::RTCStatsTimestampMaker]

    =================================================================
    ==737311==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fa597034433 bp 0x7fffe546c1c0 sp 0x7fffe546c0c0 T0)
    ==737311==The signal is caused by a READ memory access.
    ==737311==Hint: address points to the zero page.
        #0 0x7fa597034433 in mozilla::dom::RTCStatsTimestampMaker::RTCStatsTimestampMaker(mozilla::dom::GlobalObject const*) /dom/media/webrtc/jsapi/RTCStatsReport.cpp:19:53
        #1 0x7fa596ffeff3 in mozilla::PeerConnectionImpl::PeerConnectionImpl(mozilla::dom::GlobalObject const*) /dom/media/webrtc/jsapi/PeerConnectionImpl.cpp:312:7
        #2 0x7fa596ffe0bd in mozilla::PeerConnectionImpl::Constructor(mozilla::dom::GlobalObject const&) /dom/media/webrtc/jsapi/PeerConnectionImpl.cpp:237:39
        #3 0x7fa5945d2eb8 in mozilla::dom::PeerConnectionImpl_Binding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/obj-build/dom/bindings/PeerConnectionImplBinding.cpp:1779:59
        #4 0x7fa59265219e in xpc::DOMXrayTraits::construct(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&, js::Wrapper const&) /js/xpconnect/wrappers/XrayWrapper.cpp:1790:12
        #5 0x7fa59cf53ed6 in js::Proxy::construct(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /js/src/proxy/Proxy.cpp:674:19
        #6 0x7fa59cd402d8 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /js/src/vm/Interpreter.cpp:606:12
        #7 0x7fa59dcafb17 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICFallbackStub*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /js/src/jit/BaselineIC.cpp:1567:10
        #8 0x3dfde66d3e47  (<unknown module>)
    
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV /dom/media/webrtc/jsapi/RTCStatsReport.cpp:19:53 in mozilla::dom::RTCStatsTimestampMaker::RTCStatsTimestampMaker(mozilla::dom::GlobalObject const*)
    ==737311==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Detailed Crash Information

Comment 2

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20210901091701-f4ad9b76e5f8.
Failed to bisect testcase (Testcase reproduces on start build!):

Start: 00a15ff99b87cc88718646c76b48a5ea54943c52 (20200902033114)
End: f4ad9b76e5f8ca0586466fe8c8a373c6c33d0f41 (20210901091701)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False)

Comment 4

[Michael Froman [:mjf]]

I'm going to take a quick look at this locally.

Comment 5

[Michael Froman [:mjf]]

Jason, how can I try to repro this on a local build?

Comment 6

[Michael Froman [:mjf]]

I just got the correct build done here on my end. Sorry for the NI spam!

Comment 7

[Michael Froman [:mjf]]

window->GetPerformance() is returning null here: https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/RTCStatsReport.cpp#18-19

Comment 8

[Michael Froman [:mjf]]

Changing severity to S3 because this could cause a crash in the wild.

Comment 9

[Michael Froman [:mjf]]

Attachment: Bug 1728574 - make sure Performance pointer is valid before using. r?ng!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d1421d003d5
make sure Performance pointer is valid before using. r=ng

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/8d1421d003d5

Comment 12

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20210914143556-ae68c3ee95d6.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please nominate this and bug 1717318 (needed for this to apply cleanly) for ESR91 approval when you get a chance.

Comment 15

[Michael Froman [:mjf]]

Comment on attachment 9240968 [details]
Bug 1728574 - make sure Performance pointer is valid before using. r?ng!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple change, but removes chance for null dereference.
• User impact if declined: Possible crash in the wild.
• Fix Landed on Version: 94
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One line change to check for null ptr.
• String or UUID changes made by this patch: n/a

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9240968 [details]
Bug 1728574 - make sure Performance pointer is valid before using. r?ng!

Approved for 91.4esr.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/c81e8fbec9f3

Comment 18

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

:mjf, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 19

[Michael Froman [:mjf]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20210901091701-f4ad9b76e5f8.
Failed to bisect testcase (Testcase reproduces on start build!):

Start: 00a15ff99b87cc88718646c76b48a5ea54943c52 (20200902033114)
End: f4ad9b76e5f8ca0586466fe8c8a373c6c33d0f41 (20210901091701)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False)

The regression range covers 1 year (45892 commits) and says "Failed to bisect". That doesn't seem like a valid range. I'm removing [bugmon:bisected,confirmed] from the whiteboard to avoid this being flagged in the future.