1907353 - Crash in [@ mozilla::ShouldAllowAccessFor]

Status: RESOLVED FIXED

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

Description

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

8 crashes from 8 installs of Firefox 128.0, + crashes for beta and nightly builds.

Crash report: https://crash-stats.mozilla.org/report/index/02113725-88a2-4ea5-9d5c-6d6ac0240711

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  xul.dll  mozilla::ShouldAllowAccessFor(nsIChannel*, nsIURI*, unsigned int*)  toolkit/components/antitracking/StorageAccess.cpp:806
1  xul.dll  mozilla::(anonymous namespace)::ShouldPartitionChannel(nsIChannel*, nsICookie...  toolkit/components/antitracking/StoragePrincipalHelper.cpp:39
1  xul.dll  mozilla::(anonymous namespace)::ChooseOriginAttributes(nsIChannel*, mozilla::...  toolkit/components/antitracking/StoragePrincipalHelper.cpp:62
2  xul.dll  mozilla::StoragePrincipalHelper::Create(nsIChannel*, nsIPrincipal*, bool, nsI...  toolkit/components/antitracking/StoragePrincipalHelper.cpp:185
3  xul.dll  nsScriptSecurityManager::GetChannelResultStoragePrincipal(nsIChannel*, nsIPri...  caps/nsScriptSecurityManager.cpp:280
4  xul.dll  mozilla::ClearSiteData::ClearDataFromChannel(nsIHttpChannel*)  toolkit/components/clearsitedata/ClearSiteData.cpp:161
4  xul.dll  mozilla::ClearSiteData::Observe(nsISupports*, char const*, char16_t const*)  toolkit/components/clearsitedata/ClearSiteData.cpp:145
5  xul.dll  nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*)  xpcom/ds/nsObserverList.cpp:71
5  xul.dll  nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*)  xpcom/ds/nsObserverService.cpp:289
6  xul.dll  mozilla::net::nsHttpHandler::NotifyObservers(nsIChannel*, char const*)  netwerk/protocol/http/nsHttpHandler.cpp:701

Comment 1

[Tim Huang[:timhuang]]

Attachment: Bug 1907353 - Use the loadInfo to check storage access if there is no target browsing context. r?#anti-tracking!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa45bb12224b
Use the loadInfo to check storage access if there is no target browsing context. r=anti-tracking-reviewers,pbz

Comment 3

[Narcis Beleuzu [:NarcisB]]

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

Comment 4

[Tim Huang[:timhuang]]

Attachment: Bug 1907353 - Use the loadInfo to check storage access if there is no target browsing context. r?#anti-tracking!

Original Revision: https://phabricator.services.mozilla.com/D216394

Comment 5

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Firefox might crash
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: None
• Risk associated with taking this patch: Low risk
• Explanation of risk level: The patch only changes one line. The risk of taking this patch is small.
• String changes made/needed: none
• Is Android affected?: yes

Comment 6

[Tim Huang[:timhuang]]

Attachment: Bug 1907353 - Use the loadInfo to check storage access if there is no target browsing context. r?#anti-tracking!

Original Revision: https://phabricator.services.mozilla.com/D216394

Comment 7

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: Firefox might crash
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: None
• Risk associated with taking this patch: Low risk
• Explanation of risk level: The patch only changes two lines. So, it's low risk
• String changes made/needed: None
• Is Android affected?: no

Comment 8

[Tim Huang[:timhuang]]

Comment on attachment 9412725 [details]
Bug 1907353 - Use the loadInfo to check storage access if there is no target browsing context. r?#anti-tracking!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: The patch fixes a crash issue
• User impact if declined: Firefox might crash without this patch.
• Fix Landed on Version: 130
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is small.

Comment 9

[Pulsebot]

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

Comment 10

[Donal Meehan [:dmeehan]]

Comment on attachment 9412725 [details]
Bug 1907353 - Use the loadInfo to check storage access if there is no target browsing context. r?#anti-tracking!

Approved for 128.1esr.

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/f44c25ddead8

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/59d7cfbf8234