Closed Bug 1581537 Opened 7 months ago Closed 5 months ago

Browser UI locale is leaked in several ways

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: acat, Assigned: acat)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor 30683][fingerprinting])

Attachments

(1 file)

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)
Whiteboard: [tor 30683]

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

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?

Assignee: nobody → acat
Component: General → DOM: Core & HTML
Product: Firefox → Core
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

(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?

Flags: needinfo?(amarchesini)
Flags: needinfo?(MattN+bmo)
Priority: -- → P3

(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?

Flags: needinfo?(MattN+bmo)
Whiteboard: [tor 30683] → [tor 30683][fingerprinting]
Flags: needinfo?(amarchesini)
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d87c269da8a
Avoid several browser language leaks r=smaug

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

Flags: needinfo?(acat)

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".

Thanks, fixed it.

Flags: needinfo?(acat)
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ed7aa23fde5
Avoid several browser language leaks r=smaug
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Pushed by mozilla@jorgk.com:
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
You need to log in before you can comment on or make changes to this bug.