Skip to content

Bug 43330: System fonts leak when emptying the allow list on Linux.

Pier Angelo Vendrame requested to merge pierov/tor-browser:bug_43330 into tor-browser-128.5.0esr-14.5-1

Merge Info

Issues

Resolves

Related

  • #43322
  • mullvad-browser#xxxxx
  • tor-browser-build#xxxxx

Merging

Target Branches

  • tor-browser - !fixups to tor-browser-specific commits, new features, security backports
  • base-browser and mullvad-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
    • ⚠️ IMPORTANT: Please list the base-browser-specific commits which need to be cherry-picked to the base-browser and mullvad-browser branches here

Target Channels

  • Alpha: esr128-14.5
  • Stable: esr128-14.0
  • Legacy: esr115-13.5

Backporting

Timeline

  • No Backport (preferred): patchset for the next major stable
  • 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

(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
  • Localization: typos and other localization changes that should be also in the release branch
  • Other: please explain

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) : jwilde, ma1
    • fonts : pierov
    • frontend (implementation) : henry
    • frontend (review) : donuts, morgan
    • localization : henry, pierov
    • macOS : clairehurst, dan
    • nightly builds : boklm
    • rebases/release-prep : dan, ma1, pierov, morgan
    • security : jwilde, ma1
    • signing : boklm, morgan
    • updater : pierov
    • windows : jwilde, morgan
    • misc/other : pierov, morgan

Change Description

Thorin and me are evaluating the removal of font.system.whitelist, and while checking it, I discovered that my previous changes on the fontconfig configuration had some problem that weakened our protection.

However, a while ago we also defined font.system.whitelist, which worked... But I didn't remember to test without this defense in place, so I didn't realize the problem.

Before setting the FONTCONFIG_FILE variable in the browser, we used to set also FONTCONFIG_PATH. But then I defined only the file, as I understood from the docs that an absolute path was okay also without FONTCONFIG_PATH.

Seems it isn't true, so I modified the code to define both again.

How Tested

  1. Checked that emptying font.system.whitelist without the patch makes Cantarell pass (and you'll notice the chrome has different fonts). Also, TZP might detect fonts such as Cantarell, Dejavu Sans and Liberation Sans.
  2. Checked that with the patch, Cantarell isn't applied even with font.system.whitelist empty, the chrome is displayed in Arimo, and TZP is a pass.

I haven't cooked a testbuild, but I can do it if needed.

If testing with a local dev build, please make sure you have an updated start-tor-browser, or it might also contain the fontconfig variable, which might alter the tests.

Merge request reports