1623044 - Crash in [@ (anonymous namespace)::NotifyBlockingDecision] via AntiTracking

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect, P1)

Description

[Alex Chronopoulos [:achronop]]

This bug is for crash report bp-c3e590c4-37f2-4d90-b116-72c630200316.

Top 10 frames of crashing thread:

0 libxul.so  toolkit/components/antitracking/ContentBlockingNotifier.cpp:230
1 libxul.so mozilla::ContentBlockingNotifier::OnDecision toolkit/components/antitracking/ContentBlockingNotifier.cpp:447
2 libxul.so ThirdPartyUtil::AnalyzeChannel dom/base/ThirdPartyUtil.cpp:504
3 libxul.so mozilla::net::CookieServiceChild::TrackCookieLoad netwerk/cookie/CookieServiceChild.cpp:136
4 libxul.so mozilla::dom::ContentChild::UpdateCookieStatus dom/ipc/ContentChild.cpp:2092
5 libxul.so mozilla::dom::Document::StartDocumentLoad dom/base/Document.cpp:3117
6 libxul.so nsHTMLDocument::StartDocumentLoad dom/html/nsHTMLDocument.cpp:463
7 libxul.so nsContentDLF::CreateInstance layout/build/nsContentDLF.cpp:127
8 libxul.so nsDocShell::NewContentViewerObj docshell/base/nsDocShell.cpp:7872
9 libxul.so nsDocShell::CreateContentViewer docshell/base/nsDocShell.cpp:7616

Comment 1

[Andrew McCreight [:mccr8]]

The crash reason is MOZ_DIAGNOSTIC_ASSERT(aDecision == ContentBlockingNotifier::BlockingDecision::eAllow)

Comment 2

[Steven Englehardt [:englehardt]]

Ehsan, are you able to look at this?

Comment 3

[Pascal Chevrel:pascalc]

This is spiking on Nightly since build 20200325213906

Comment 4

[Steven Englehardt [:englehardt]]

Redirecting to baku since Ehsan is out for a bit.

Comment 5

[Dimi Lee [:dimi]]

This should be related to changes recently made for fission, I'll check it

Comment 6

[Wayne Mery (:wsmwk)]

This is a topcrash for Thunderbird daily. I can't speculate on Firefox

Comment 7

[Dimi Lee [:dimi]]

The root cause of this bug is that when opening the devtools (about:devtools-toolbox), the about page may also embed a 3rd-party frame with chrome URL, ex, chrome://devtools/content/debugger/index.html.

Although the frame has system privilege and it is not a tracker, we may still deny the storage access permission when setting cookiebehavior to REJCET ALL, which trigger the MOZ_ASSERT here that asserts while notifying blocking events for windows with system principal.

baku, steven,
I would like to check with you do you think we should always allow storage permission for system privileged windows no matter what cookiebehavior is set? or we should respect cookiebehavior for system content.
If the latter case is the case we want, the solution for this bug will be not notifying content blocking events when we block storage access for system content, because no UX requires that info.

Comment 8

[Andrea Marchesini [:baku]]

If the latter case is the case we want, the solution for this bug will be not notifying content blocking events when we block storage access for system content, because no UX requires that info.

I prefer this. What I want to avoid is an about page, with privileged permissions, to run a webpage. That webpage should have the correct storage permissions.

Comment 9

[Dimi Lee [:dimi]]

Attachment: Bug 1623044 - Remove MOZ_ASSERT when notify blocking decision on a system privileged window r=baku

Comment 10

[Dimi Lee [:dimi]]

Attachment: Bug 1623044 - Remove MOZ_ASSERT when notifying blocking decision on a system privileged window r=baku

Comment 11

[Steven Englehardt [:englehardt]]

(In reply to Andrea Marchesini [:baku] from comment #8)

If the latter case is the case we want, the solution for this bug will be not notifying content blocking events when we block storage access for system content, because no UX requires that info.

I prefer this. What I want to avoid is an about page, with privileged permissions, to run a webpage. That webpage should have the correct storage permissions.

This sounds right to me as well. Thanks Baku and Dimi!

Comment 12

[Dimi Lee [:dimi]]

Attachment: Bug 1623044 - Allow list checks should return allow for a system window. r=timhuang

Comment 13

[Dimi Lee [:dimi]]

talked offline with baku, since the behavior before Bug 1612378 is always allowing permission for system window, we'll follow it.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8e724edea2a
Allow list checks should return allow for a system window. r=timhuang,baku

Comment 15

[Cosmin Sabou [:CosminS]]

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