1713475 - Firefox crashes on invalid privacy.restrict3rdpartystorage.skip_list setting

Status: RESOLVED FIXED

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

Description

[Michael Hoffmann]

Steps to reproduce:

1. Go to about:config.
2. Create a privacy.restrict3rdpartystorage.skip_list string and input ; as its value.
3. Firefox crashes.

The crash happens while executing PartitioningExceptionList::OnExceptionListUpdate since the it does not properly handle invalid input: https://searchfox.org/mozilla-central/source/toolkit/components/antitracking/PartitioningExceptionList.cpp#124-150

Comment 1

[Michael Hoffmann]

I wrote a patch to fix this. I'll check if I can remember how to get it into Phabricator.

Comment 2

[Michael Hoffmann]

Attachment: Bug 1713475 - Don't crash on invalid privacy.restrict3rdpartystorage.skip_list

Comment 3

[Michael Hoffmann]

Thanks for the review! Could someone please land the fix, as I don't have the rights to do it myself.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7da256f6d3b3
Don't crash on invalid privacy.restrict3rdpartystorage.skip_list r=timhuang

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7da256f6d3b3

Comment 6

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

The patch landed in nightly and beta is affected.
:brennan.brisad, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

[Michael Hoffmann]

Comment on attachment 9224535 [details]
Bug 1713475 - Don't crash on invalid privacy.restrict3rdpartystorage.skip_list

Beta/Release Uplift Approval Request

• User impact if declined: An user wanting to add an exemption to State Partitioning by following the instructions here https://developer.mozilla.org/en-US/docs/Web/Privacy/State_Partitioning#exempt_specific_origins_from_partitioning, can simply mistype the syntax and instantly crash Firefox.
• 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): The change is only adding two simple conditionals to avoid dereferencing invalid pointers.
• String changes made/needed: none

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 9224535 [details]
Bug 1713475 - Don't crash on invalid privacy.restrict3rdpartystorage.skip_list

AFAICT this isn't often hit in the wild, so I prefer to let it ride the trains.

Comment 9

[Julien Cristau [:jcristau]]

One crash report I noticed that looks like it hit this bug is bp-b0524b53-f245-4fe2-b114-3cbcd0210513