1581537 - Browser UI locale is leaked in several ways

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Alex Catarineu (Tor Browser dev)]

This is https://trac.torproject.org/projects/tor/ticket/30683 in Tor Browser.

Browser UI locale can be obtained in several ways:

• dom/dom.properties (e.g. getting aForm.validationMessage).
• layout/xmlparser.properties ​ (e.g. getting DOMParser error messages)
• layout/MediaDocument.properties (e.g. getting contentWindow.document.title of iframe which has an image as src)

Comment 1

[Alex Catarineu (Tor Browser dev)]

Attachment: Bug 1581537 - Avoid several browser language leaks

Spoof dom/dom.properties, layout/xmlparser.properties,
layout/MediaDocument.properties to en-US if needed.

Comment 2

[Axel Hecht [:Pike]]

The patch as written will impact form validation in all UI built into Firefox. Notably about:logins, but that's also basically everything I checked.

People using localized builds w/ fingerprinting on will see English form validation messages.

Not sure if there's other impact on Firefox UI in this patch.

Who can help make a call on what's OK here?

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Axel Hecht [:Pike] from comment #2)

The patch as written will impact form validation in all UI built into Firefox. Notably about:logins, but that's also basically everything I checked.

People using localized builds w/ fingerprinting on will see English form validation messages.

Not sure if there's other impact on Firefox UI in this patch.

Who can help make a call on what's OK here?

:baku and :MattN can you please shed some light here?

Comment 4

[Matthew N. [:MattN]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #3)

(In reply to Axel Hecht [:Pike] from comment #2)

The patch as written will impact form validation in all UI built into Firefox. Notably about:logins, but that's also basically everything I checked.

That's correct. It would affect at least about:logins and the Form Autofill dialogs at about:preferences#privacy-form-autofill. Both of those are used in documents without the system principal (the latter is loaded with the system principal in preferences but not when also loaded in the Web Payments dialog).

People using localized builds w/ fingerprinting on will see English form validation messages.

It would also be nice if we used nsContentUtils::PrincipalAllowsL10n to allow seeing the localized .validationMessage string so that our validation popup UI and tooltips remain localized.

Not sure if there's other impact on Firefox UI in this patch.

Who can help make a call on what's OK here?

I'm not sure of all the existing trade-offs we've may have already accepted for similar patches but it seems like we should at least align with bug 467035.

:baku and :MattN can you please shed some light here?

How hard would it be to make an exception for built-in pages using nsContentUtils::PrincipalAllowsL10n like we did for bug 467035. I don't see why we wouldn't be consistent between these two fixes. Does that cover moz-extension:// too?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d87c269da8a
Avoid several browser language leaks r=smaug

Comment 6

[Alexandru Michis [:malexandru]]

Backed out changeset 1d87c269da8a for causing bc failures in browser_misused_characters_in_strings.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/16931240f0f1a86dfc5ce68d0ecba21f1fde736d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&tochange=16931240f0f1a86dfc5ce68d0ecba21f1fde736d&fromchange=1d87c269da8a2df75b09d475cbac1c310398eb6d&selectedJob=274380695

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274380695&repo=autoland&lineNumber=1568

[task 2019-11-04T13:21:44.522Z] 13:21:44 INFO - TEST-PASS | browser/base/content/test/static/browser_misused_characters_in_strings.js | Found 144 .properties files to scan for misused characters -
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - Buffered messages finished
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_misused_characters_in_strings.js | jar:file:///Users/cltbld/tasks/task_1572873431/build/application/Firefox%20Nightly.app/Contents/Resources/omni.ja!/res/locale/dom/dom.properties with key=PatternAttributeCompileFailure has a misused single-quote. Single-quoted strings should use Unicode ‘foo’ instead of 'foo'. -
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - Stack trace:
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - chrome://mochikit/content/browser-test.js:test_ok:1299
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_misused_characters_in_strings.js:testForError:145
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_misused_characters_in_strings.js:testForErrors:166
[task 2019-11-04T13:21:44.523Z] 13:21:44 INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_misused_characters_in_strings.js:checkAllTheProperties:222
[task 2019-11-04T13:21:44.524Z] 13:21:44 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1069
[task 2019-11-04T13:21:44.524Z] 13:21:44 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1104
[task 2019-11-04T13:21:44.524Z] 13:21:44 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:932
[task 2019-11-04T13:21:44.524Z] 13:21:44 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:805
[task 2019-11-04T13:21:44.524Z] 13:21:44 INFO - Leaving test bound checkAllTheProperties

Comment 7

[:Gijs (he/him)]

The test failure is because

https://searchfox.org/mozilla-central/rev/35873cfc312a6285f54aa5e4ec2d4ab911157522/browser/base/content/test/static/browser_misused_characters_in_strings.js#97-101

  {
    file: "dom.properties",
    key: "PatternAttributeCompileFailure",
    type: "single-quote",
  },

dom.properties has a string that would ordinarily be required to use curly quotes. The test removes entries from the list of exceptions when it encounters them, and because we now package this file twice, the second time it encounters the entry, there is no exception in the list anymore so it fails.

The simplest fix would be to duplicate the exception entry I cited, probably with a comment along the lines of "dom.properties is packaged twice so we need to have 2 exceptions for this string".

Comment 8

[Alex Catarineu (Tor Browser dev)]

Thanks, fixed it.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ed7aa23fde5
Avoid several browser language leaks r=smaug

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/0ed7aa23fde5

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b2f5e2c9055b
Port bug 1581537 - Add some dom and layout properties files to the package manifest. rs=bustage-fix