1942517 - Crash in [@ mozilla::dom::WorkerPrivate::GetCSPInfo]

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P3)

Description

[Donal Meehan [:dmeehan]]

Crash report: https://crash-stats.mozilla.org/report/index/1d984d0e-8d7f-4968-bfd1-1d1d60250119

Reason:

EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  xul.dll  mozilla::UniquePtr<mozilla::ipc::CSPInfo, mozilla::DefaultDelete<mozilla::ipc...  mfbt/UniquePtr.h:287
0  xul.dll  mozilla::UniquePtr<mozilla::ipc::CSPInfo, mozilla::DefaultDelete<mozilla::ipc...  mfbt/UniquePtr.h:278
0  xul.dll  mozilla::dom::WorkerPrivate::GetCSPInfo() const  dom/workers/WorkerPrivate.h:937
0  xul.dll  mozilla::dom::TrustedTypeUtils::GetTrustedTypesCompliantString(mozilla::dom::...  dom/security/trusted-types/TrustedTypeUtils.cpp:489
0  xul.dll  mozilla::dom::TrustedTypeUtils::GetTrustedTypesCompliantString(mozilla::dom::...  dom/security/trusted-types/TrustedTypeUtils.cpp:552
1  xul.dll  mozilla::dom::DOMParser::ParseFromString(mozilla::dom::TrustedHTMLOrString co...  dom/base/DOMParser.cpp:110
1  xul.dll  mozilla::dom::DOMParser_Binding::parseFromString(JSContext*, JS::Handle<JSObj...  dom/bindings/DOMParserBinding.cpp:97
2  xul.dll  mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::Nor...  dom/bindings/BindingUtils.cpp:3290
3  xul.dll  CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::...  js/src/vm/Interpreter.cpp:532
3  xul.dll  js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstru...  js/src/vm/Interpreter.cpp:628

Comment 1

[Donal Meehan [:dmeehan]]

:iain is it possible this is a regression from Bug 1932864?

Comment 2

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

Fred, could you take a look at this regression from bug 1901492?

Comment 3

[Frédéric Wang (:fredw)]

Yes, from the backtrace this is likely due to this change.

I had checked callers in https://phabricator.services.mozilla.com/D233507?id=967939#inline-1302263

and DOMParser was supposed to be only used in a Window global scope: https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#domparser

Maybe this is an edge case when DOMParser::mOwner is actually null and we wrongly go to the else branch for workers https://searchfox.org/mozilla-central/rev/9a66d18cb35595c89f499a1011c9dd7e573fce77/dom/security/trusted-types/TrustedTypeUtils.cpp#489 (in the past this was just returning early and so hiding the problem).

Do we actually have a test case we can use to reproduce the issue?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

DOMParser can be used outside Window, in those system JS globals what I mentioned on Matrix.

https://searchfox.org/mozilla-central/search?q=new+DOMParser%28&path=&case=false&regexp=false

I guess the crash happens only if one has dom.security.trusted_types.enabled enabled?

Comment 6

[Frédéric Wang (:fredw)]

Thanks, I'll check more.

Yes, the affected code only runs dom.security.trusted_types.enabled enabled.

Comment 7

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

The bug is marked as tracked for firefox136 (nightly). We have limited time to fix this, the soft freeze is in 8 days. However, the bug still isn't assigned.

:freddy, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 8

[Frédéric Wang (:fredw)]

Attachment: Bug 1942517 - Skip trusted type check for DOMParser when global object is not a Window. r=smaug

Comment 9

[Frederik Braun [:freddy]]

Downranking Priority as this is only happening for. builds that have trusted types enabled (off by default).

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3b4d635577f
Skip trusted type check for DOMParser when global object is not a Window. r=smaug

Comment 11

[Serban Stanca [:SerbanS]]

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

Comment 12

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

Set release status flags based on info from the regressing bug 1901492