Skip to content

Draft: Bug 42163: Make the DLL blocklist obey portable mode

Merge Info

Related Issues

Backporting

Timeline

  • Immediate: patchset needed as soon as possible
  • Next Minor Stable Release: patchset that needs to be verified in nightly before backport
  • Eventually: patchset that needs to be verified in alpha before backport
  • No Backport (preferred): patchset for the next major stable

The MR is currently targeting 13.0 because we don't have a 13.5 branch. However, I think we could start merging to 13.5 instead of 13.0.

(Optional) Justification

  • Emergency security update: patchset fixes CVEs, 0-days, etc
  • Censorship event: patchset enables censorship circumvention
  • Critical bug-fix: patchset fixes a bug in core-functionality
  • Consistency: patchset which would make development easier if it were in both the alpha and release branches; developer tools, build system changes, etc
  • Sponsor required: patchset required for sponsor
  • Other: please explain

Merging

  • Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports
  • Merge to base-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
    • NOTE: if your changeset includes patches to both base-browser and tor-browser please clearly label in the change description which commits should be cherry-picked to base-browser after merging

Issue Tracking

Review

Request Reviewer

  • Request review from an applications developer depending on modified system:
    • NOTE: if the MR modifies multiple areas, please /cc all the relevant reviewers (since gitlab only allows 1 reviewer)
    • accessibility : henry
    • android : clairehurst, dan
    • build system : boklm
    • extensions : ma1
    • firefox internals (XUL/JS/XPCOM) : ma1
    • fonts : pierov
    • frontend (implementation) : henry
    • frontend (review) : donuts, richard
    • localization : henry, pierov
    • macos : clairehurst, dan
    • nightly builds : boklm
    • rebases/release-prep : dan, ma1, pierov, richard
    • security : ma1
    • signing : boklm, richard
    • updater : pierov
    • misc/other : pierov, richard

Change Description

Firefox 110 introduced the possibility of blocking third-party injected DLLs.

It happens in an early stage of the startup, so it isn't on a profile basis but on an installation basis. For this reason, it skips our portable mode patch, which isn't great.

13.0 alphas added a file to %appData%\Tor Project\Firefox, but only when you first block a DLL in about:third-party.

With this MR, we add the file to Browser\TorBrowser\Data\Browser, instead.

However, to make this possible I needed to manipulate paths in C (widechar) strings. Since we're in an early phase and in firefox.exe, we don't have the usual XPCOM utilities, so I used Win32 Path*W functions, instead of normal (wide) string functions, but this required me to add the related Win32 library (shouldn't be a problem, as it's an OS library that has been there since Windows 2000).

An interesting fact is that the relative profile directory must have forward slashes (we use nsIFile::SetRelativePath, which works with UTF-8 paths with forward slashes, not native paths). Win32 API works well also with forward slashes, but the following Firefox functions don't 😂. So, I had to implement a string replace, because from what I can tell, Win32 doesn't have a function to normalize slashes and the C/C++ standard libraries don't have a function to replace all the occurrences of a character (unless I used std::replace from <algorithm>, but it needs to have an end iterator... which we should have at compile time with sizeof(path) / sizeof(wchar_t) being a literal/stack buffer and without using wcslen, let me know if you prefer this solution).

I haven't implemented a migration/cleaning function. This is a new feature and I think it won't be a big deal if we don't have one.

Finally, this path used to be written in the registry. Firefox's algorithm is like:

  1. check for a path in the registry, if you have it, return it immediately
  2. otherwise, build the default path (the function we patch)
  3. write the default path to the registry and return it

We have an issue only for registry entries (#41891), but I preferred removing this entry already in this patch.

I considered leaving the code to read an existing entry, but then I disabled it anyway because in the moment you want to read a key, you have to create all the parents one, from what I understand (at least in Firefox's implementation).

How Tested

You might need:

  • Test build (created with an earlier version that didn't have a couple of comments in the moz.build files, but the code is the same)
  • Bonjour Print Services for Windows if you don't see anything in about:third-party and you need an easy way of adding something to it

After that:

  • Go to about:third-party, click the button to enable system data (it might take a while to appear) and block a DLL. Restart the browser and check that the DLL stays disabled (the symbol in the button is a cross in a red circle, and the tooltip tells you it's currently disabled). Notice that Browser\TorBrowser\Data\Browser\blocklist has been created.
  • Close the browser, delete Browser\TorBrowser\Data\Browser\blocklist, go to about:third-party again and notice the DLL has been unblocked
  • Check that HKEY_CURRENT_USER\Software\Tor Project\Firefox\Launcher doesn't have the blocklist entry for this installation of Tor Browser (you can compare with other 13.0aX installs)
Edited by Pier Angelo Vendrame

Merge request reports