1744362 - Introduce a button to temporarily block an injected module in about:third-party

Status: RESOLVED FIXED

(Firefox :: Launcher Process, enhancement)

Description

[Toshihito Kikuchi [:toshi]]

The current about:third-party page displays read-only contents only. Our next step is to introduce a button on the page so that users can block unwanted modules on their own.

Comment 1

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

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 2

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.

Comment 3

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 1: Extract macro definitions from WindowsDllBlocklistCommon.h

Comment 4

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 2: Minor fixes

• AddString should check .Length instead of .MaximumLength
• SharedSection::Init does not have to take PEHeaders
• Improve the boundary check in SharedSection::AddDependentModule
• Use MOZ_LITERAL_UNICODE_STRING in Kernel32ExportsSolver::ResolveInternal
• Use SharedSection::ConvertToReadOnly in TestCrossProcessWin
• Use SharedSection::Reset to close the handle in TestCrossProcessWin
• Typo: s/AddDepenentModule/AddDependentModule/
• TestCrossProcessWin should compare leaf names

Depends on D159202

Comment 5

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 3: Hide SharedSection::Layout

Depends on D159203

Comment 6

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 4: Move mState from Kernel32ExportsSolver to Layout

Depends on D159204

Comment 7

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 5: Access the shared section through DllServices

Depends on D159205

Comment 8

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 6: Add DynamicBlocklist

Depends on D159206

Comment 9

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 7: Add the block buttons to about:third-party

Depends on D159207

Comment 10

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 8: Store a blocklist's path to the registry

Depends on D159208

Comment 11

[Haik Aftandilian [:haik]]

Attachment: WIP: Bug 1744362 - Part 9: Introduce AboutThirdParty::GetIsDynamicBlocklistAvailable

Depends on D159209

Comment 12

[Haik Aftandilian [:haik]]

I've posted Toshi's patch set for this work from try revision 570165fbc6ef so we're less likely to lose the patches. These were from March, 2022.

Comment 13

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 1: Extract macro definitions from WindowsDllBlocklistCommon.h r=handyman

Comment 14

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 2: Minor fixes r=handyman

• AddString should check .Length instead of .MaximumLength
• SharedSection::Init does not have to take PEHeaders
• Improve the boundary check in SharedSection::AddDependentModule
• Use MOZ_LITERAL_UNICODE_STRING in Kernel32ExportsSolver::ResolveInternal
• Use SharedSection::ConvertToReadOnly in TestCrossProcessWin
• Use SharedSection::Reset to close the handle in TestCrossProcessWin
• Typo: s/AddDepenentModule/AddDependentModule/
• TestCrossProcessWin should compare leaf names

Depends on D164483

Comment 15

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 3: Hide SharedSection::Layout r=handyman

Depends on D164484

Comment 16

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 4: Move mState from Kernel32ExportsSolver to Layout r=handyman

Depends on D164485

Comment 17

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 5: Access the shared section through DllServices r=handyman

Depends on D164486

Comment 18

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 6: use dynamic blocklist file to block third-party DLLs r=handyman

Depends on D164487

Comment 19

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 7: support code for about:third-party r=handyman

Depends on D164488

Comment 20

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 8: allow blocking of third-party DLLs on about:third-party r=Gijs,fluent-reviewers,flod

Depends on D164489

Comment 21

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 9: update Enterprise policies schema r=mkaply,fluent-reviewers,flod

Depends on D164490

Comment 22

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 10: update documentation r=handyman

This pulls in the existing wiki page about the blocklist from https://wiki.mozilla.org/Blocklisting/DLL and adds some technical details and information about the new dynamic blocklist.

Depends on D164491

Comment 23

[Greg Stoll :gstoll]

Attachment: about:third-party with new "block" buttons, but nothing blocked

about:third-party with new "block" buttons, but nothing blocked

Comment 24

[Greg Stoll :gstoll]

Attachment: about:third-party after choosing to block TortoiseOverlays.dll

about:third-party after choosing to block TortoiseOverlays.dll

Comment 25

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 11: make dynamic blocklist work on DLLs that load before kernel32 r=handyman

Two of the three DLLs that Avast injects into Firefox were not properly being blocked when on the dynamic blocklist because they were being loaded before kernel32.dll, and SharedSection::Layout::Resolve() would fail. For the dynamic blocklist part of things we don't actually need the kernel32 exports, so this change moves them to the end of Resolve() and adds an intermediate state where the dynamic blocklist entries have been loaded but not the kernel32 exports. Now all three DLLs can be blocked correctly when on the dynamic blocklist.

Depends on D164492

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1dd7e7c111da
Part 1: Extract macro definitions from WindowsDllBlocklistCommon.h r=handyman
https://hg.mozilla.org/integration/autoland/rev/704a4150d210
Part 2: Minor fixes r=handyman
https://hg.mozilla.org/integration/autoland/rev/438f74bb5b7c
Part 3: Hide SharedSection::Layout r=handyman
https://hg.mozilla.org/integration/autoland/rev/c88902b60d1f
Part 4: Move mState from Kernel32ExportsSolver to Layout r=handyman
https://hg.mozilla.org/integration/autoland/rev/80b67f7ea109
Part 5: Access the shared section through DllServices r=handyman
https://hg.mozilla.org/integration/autoland/rev/acfdb2bcaa9f
Part 6: use dynamic blocklist file to block third-party DLLs r=handyman
https://hg.mozilla.org/integration/autoland/rev/848632184f56
Part 7: support code for about:third-party r=handyman
https://hg.mozilla.org/integration/autoland/rev/8b50446f91e5
Part 8: allow blocking of third-party DLLs on about:third-party r=Gijs,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/8c87571dbb3e
Part 9: update Enterprise policies schema r=mkaply,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/cf1db931c466
Part 10: update documentation r=handyman
https://hg.mozilla.org/integration/autoland/rev/4b2e3689cea0
Part 11: make dynamic blocklist work on DLLs that load before kernel32 r=handyman

Comment 27

[Serban Stanca [:SerbanS]]

Backed out for causing multiple failures and build bustages.

• Backout link

• Push with failures

• Failure Log

• Failure line: TEST-UNEXPECTED-FAIL | dom/canvas/test/webgl-conf/generated/test_2_conformance2__textures__misc__tex-3d-size-limit.html | application terminated with exit code 1 | after SUMMARY: AddressSanitizer: access-violation /builds/worker/checkouts/gecko/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp:455 in mozilla::GetPrespawnCigExceptionModules::<lambda_3>::operator()

• It also failed on this.

• Failure line: TEST-UNEXPECTED-FAIL | telemetry/marionette/tests/client/test_shutdown_pings_succeed.py TestShutdownPingsSucced.test_shutdown_pings_succeed_pingsender2 | AssertionError: {'event': b'2.0', 'first-shutdown': b'2.0', 'main': b'', 'new-profile': b'2.0'} != {'first-shutdown': b'', 'main': b'', 'event': b'', 'new-profile': b''}

• Failure Log

• It caused build bustages too --> https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=ab4dceb3ca5172cfe87aa7570339e1e51ec1412e&tochange=81e46b9ec13718e779669e1939817b743d5399c3&selectedTaskRun=YFn4HvgNT_ezyntUdz8WQw.0 and --> https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=ab4dceb3ca5172cfe87aa7570339e1e51ec1412e&tochange=81e46b9ec13718e779669e1939817b743d5399c3&selectedTaskRun=aTDWJhTYQxKG4wmyqTvgBQ.0

Comment 28

[Serban Stanca [:SerbanS]]

• It also caused xpcshell failures --> https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&searchStr=windows%2C10%2Cx64%2C2004%2Cwebrender%2Copt%2Cxpcshell%2Ctests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-windows10-64-2004-qr%2Fopt-xpcshell-spi-nw%2Cx3&fromchange=ab4dceb3ca5172cfe87aa7570339e1e51ec1412e&tochange=81e46b9ec13718e779669e1939817b743d5399c3&selectedTaskRun=MKcUt9mpRK2IvP6hzrIhww.0.
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/components/backgroundtasks/tests/xpcshell/test_backgroundtask_unique_profile.js | xpcshell return code: 0

Comment 29

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 12: fix build and some tests r=handyman

• In sandboxBroker.cpp Be more careful about checking whether GetDependentModules() is returning an empty span to avoid ASAN problems
• In TestCrossProcessWin.cpp, make UniquePtr live as long as the Span that wraps it
• In LauncherRegistryInfo, mingw doesn't allow using constexpr with expressions containing '|', so just make flags const instead.

Depends on D164738

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab1292118c00
Part 1: Extract macro definitions from WindowsDllBlocklistCommon.h r=handyman
https://hg.mozilla.org/integration/autoland/rev/0a3ce8ea039e
Part 2: Minor fixes r=handyman
https://hg.mozilla.org/integration/autoland/rev/d1f4b99f352b
Part 3: Hide SharedSection::Layout r=handyman
https://hg.mozilla.org/integration/autoland/rev/2911fe484cc3
Part 4: Move mState from Kernel32ExportsSolver to Layout r=handyman
https://hg.mozilla.org/integration/autoland/rev/401ebbc0159d
Part 5: Access the shared section through DllServices r=handyman
https://hg.mozilla.org/integration/autoland/rev/a2d8f526e0ff
Part 6: use dynamic blocklist file to block third-party DLLs r=handyman
https://hg.mozilla.org/integration/autoland/rev/315c57948afa
Part 7: support code for about:third-party r=handyman
https://hg.mozilla.org/integration/autoland/rev/da9133df4cd4
Part 8: allow blocking of third-party DLLs on about:third-party r=Gijs,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/39ff51df4a45
Part 9: update Enterprise policies schema r=mkaply,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/34f51e6aee96
Part 10: update documentation r=handyman
https://hg.mozilla.org/integration/autoland/rev/0df403e8f6ba
Part 11: make dynamic blocklist work on DLLs that load before kernel32 r=handyman
https://hg.mozilla.org/integration/autoland/rev/3f63f21115e2
Part 12: fix build and some tests r=handyman

Comment 31

[Greg Stoll :gstoll]

landed changes that should fix these

Comment 32

[Atila Butkovits]

Backed out for causing MinGW build bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/2970d380a628ca203cd63797601f5945dd189a17

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=3f63f21115e28d9cc2244984941105eb31d21a99&selectedTaskRun=P737eBVxR3y7UzKbR2j2Hg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=400843008&repo=autoland&lineNumber=98226

Comment 33

[Greg Stoll :gstoll]

Attachment: Bug 1744362 - Part 13: special-case loading kernel32.dll to avoid crashes r=handyman

The comment mostly explains it. I don't understand why this only happens on PGO builds, but it's 100% reproducible on try builds and the fix seems reasonable enough.

Depends on D165561

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5105bbfbd67d
Part 1: Extract macro definitions from WindowsDllBlocklistCommon.h r=handyman
https://hg.mozilla.org/integration/autoland/rev/3ea314484db7
Part 2: Minor fixes r=handyman
https://hg.mozilla.org/integration/autoland/rev/f0e4294e5bc2
Part 3: Hide SharedSection::Layout r=handyman
https://hg.mozilla.org/integration/autoland/rev/7616ecd9e3f6
Part 4: Move mState from Kernel32ExportsSolver to Layout r=handyman
https://hg.mozilla.org/integration/autoland/rev/1685365fd68d
Part 5: Access the shared section through DllServices r=handyman
https://hg.mozilla.org/integration/autoland/rev/f2d8658ce1b1
Part 6: use dynamic blocklist file to block third-party DLLs r=handyman
https://hg.mozilla.org/integration/autoland/rev/9a2bcd63febe
Part 7: support code for about:third-party r=handyman
https://hg.mozilla.org/integration/autoland/rev/59635f1ed0bd
Part 8: allow blocking of third-party DLLs on about:third-party r=Gijs,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/bf477ea61809
Part 9: update Enterprise policies schema r=mkaply,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/5bc146059945
Part 10: update documentation r=handyman
https://hg.mozilla.org/integration/autoland/rev/17351698e3e7
Part 11: make dynamic blocklist work on DLLs that load before kernel32 r=handyman
https://hg.mozilla.org/integration/autoland/rev/0524114dbd0e
Part 12: fix build and some tests r=handyman
https://hg.mozilla.org/integration/autoland/rev/c3b81880d219
Part 13: special-case loading kernel32.dll to avoid crashes r=handyman

Comment 35

[Greg Stoll :gstoll]

landed changes (again :-) ) that should fix these

Comment 36

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5105bbfbd67d
https://hg.mozilla.org/mozilla-central/rev/3ea314484db7
https://hg.mozilla.org/mozilla-central/rev/f0e4294e5bc2
https://hg.mozilla.org/mozilla-central/rev/7616ecd9e3f6
https://hg.mozilla.org/mozilla-central/rev/1685365fd68d
https://hg.mozilla.org/mozilla-central/rev/f2d8658ce1b1
https://hg.mozilla.org/mozilla-central/rev/9a2bcd63febe
https://hg.mozilla.org/mozilla-central/rev/59635f1ed0bd
https://hg.mozilla.org/mozilla-central/rev/bf477ea61809
https://hg.mozilla.org/mozilla-central/rev/5bc146059945
https://hg.mozilla.org/mozilla-central/rev/17351698e3e7
https://hg.mozilla.org/mozilla-central/rev/0524114dbd0e
https://hg.mozilla.org/mozilla-central/rev/c3b81880d219

Comment 37

[Pascal Chevrel:pascalc]

Greg, is that something that needs a mention in our release notes for 110? Thanks

Comment 38

[Greg Stoll :gstoll]

I think it would be nice to mention in the release notes - :haik, do you agree?

Comment 39

[Haik Aftandilian [:haik]]

(In reply to Greg Stoll from comment #38)

I think it would be nice to mention in the release notes - :haik, do you agree?

Yep, definitely.

Comment 40

[Greg Stoll :gstoll]

Release Note Request (optional, but appreciated)
[Why is this notable]: Allows users to block third-party modules that inject themselves into the Firefox process.
[Affects Firefox for Android]: No
[Suggested wording]: On Windows, third-party modules can now be blocked from injecting themselves into Firefox, which can be helpful if they are causing crashes or other undesirable behavior. See this page for more information. (with link to the SUMO page below)
[Links (documentation, blog post, etc)]: https://support.mozilla.org/en-US/kb/identify-problems-third-party-modules-firefox-windows (this page has not been updated yet)

Comment 41

[Pascal Chevrel:pascalc]

Note added to our nightly and beta 110 release notes (without the link until the content is updated).

Comment 42

[Pascal Chevrel:pascalc]

Added to final 110 notes.