Properties in dom/locales/$lang/chrome/ allow detecting user locale
z3t reported a bunch of issues on HackerOne regarding detection of user locale with the help of dom/locales/$lang/chrome/ properties. PoCs done by z3t:
dom/dom.properties: https://people.torproject.org/~gk/tests/tor_form_locale_leak.html
layout/xmlparser.properties: https://people.torproject.org/~gk/tests/tor_domparser_locale_leak.html
layout/MediaDocument.properties: https://people.torproject.org/~gk/tests/tor_image_locale_leak.html
Activity
Replying to gk:
z3t reported a bunch of issues on HackerOne regarding detection of user locale with the help of
dom/locales/$lang/chrome/properties. PoCs done by z3t:dom/dom.properties: https://people.torproject.org/~gk/tests/tor_form_locale_leak.html<input type="email">is relevant here as well (thanks to mik317 on HackerOne for pointing that out).Here is a small patch for review: https://www.github.com/acatarineu/tor-browser/commit/30683. I checked htmlparser.properties just in case, but it seems only being used in
view-[*](*`) pages, which should not be accessible from web content.Do we want all these
tbb-fingerprinting-localefixed just for next esr68 or also for current esr60? I did the patch based on current alpha, but it should not be difficult to adapt for esr68.Trac:
Keywords: tbb-fingerprinting-locale deleted, tbb-fingerprinting-locale TorBrowserTeam201906R added
Status: new to needs_review-
Replying to acat:
Here is a small patch for review: https://www.github.com/acatarineu/tor-browser/commit/30683. I checked htmlparser.properties just in case, but it seems only being used in
view-[*](*`) pages, which should not be accessible from web content.Do we want all these
tbb-fingerprinting-localefixed just for next esr68 or also for current esr60? I did the patch based on current alpha, but it should not be difficult to adapt for esr68.We need a patch for esr68 as well be it now or when we rebase, so having one version for esr68 (even though we start testing with an esr60 alpha) seems good. Ideally, while you are at it you'd file a ticket at bugzilla and get the patch upstreamed (and with some luck we can include it that way in the esr68 without needing to ship the patch ourselves).
-
I think this looks mostly good to me. Could you adapt
// When we spoof English, use en-US default strings in HTML forms.a bit as the code that follows is not only about forms anymore?Have you checked whether the patch is a good upstreaming idea as-is? I wonder in particular if the
en-USstrings will always be available in, sayfr, builds that don't come withfrlang packs.mcs, brade could you have a second look?
Trac:
Cc: N/A to mcs, brade
Keywords: TorBrowserTeam201907R deleted, TorBrowserTeam201907 added
Status: needs_review to needs_revision Have you checked whether the patch is a good upstreaming idea as-is? I wonder in particular if the en-US strings will always be available in, say fr, builds that don't come with fr lang packs.
That's a very nice point, I missed this one. Indeed, Firefox localized builds do not have these, I only tested with language packs... Good that you realized before https://phabricator.services.mozilla.com/D35815 landed, since it was accepted.
I'm not sure what's the best way to make this upstreamable. I think it's not realistic to try to make the localized messages not accessible to web content, so I think making available these en-US locales in localized builds is the only way. I will take a look and try to see how difficult it is to do this, and update https://bugzilla.mozilla.org/show_bug.cgi?id=1561322. The approach is the same, so whatever is accepted in that bug should also apply here.
Replying to gk:
I think this looks mostly good to me. Could you adapt
// When we spoof English, use en-US default strings in HTML forms.a bit as the code that follows is not only about forms anymore?Maybe something like:
// When we spoof English, use en-US properties in strings that are accessible by content.Have you checked whether the patch is a good upstreaming idea as-is? I wonder in particular if the
en-USstrings will always be available in, sayfr, builds that don't come withfrlang packs.mcs, brade could you have a second look?
r=brade,r=mcs The patch looks good to us. Regarding upstreaming, it seems like the choice is to include the en-US properties files or hard-coded a bunch of strings. Including the en-US files seems like a good solution, but I guess we will see what the Mozilla engineers say.
Here is a branch with the patch ported to esr68: https://github.com/acatarineu/tor-browser/commits/30683_esr68. I also reverted #24056 (moved) and backported the upstreamed to Firefox one, since this patch depends on those changes. So #31298 (moved) should be also fixed here.
Trac:
Keywords: TorBrowserTeam201908 deleted, TorBrowserTeam201908R added
Status: needs_revision to needs_reviewMore than porting, it's actually rewriting the patch similarly to the upstreamed patch for #24056 (moved) (https://bugzilla.mozilla.org/show_bug.cgi?id=1561322).
-
Replying to acat:
More than porting, it's actually rewriting the patch similarly to the upstreamed patch for #24056 (moved) (https://bugzilla.mozilla.org/show_bug.cgi?id=1561322).
It seems my tiny request in comment:6 still applies?
Trac:
Keywords: TorBrowserTeam201908R deleted, TorBrowserTeam201908 added
Status: needs_review to needs_revision Yes, fixed in https://github.com/acatarineu/tor-browser/commits/30683_esr68+1.
Trac:
Status: needs_revision to needs_reviewFixup for Android in https://github.com/acatarineu/tor-browser/commit/30683_esr68+2.
Replying to gk:
Looks good. I wonder whether the ServiceWorker
ReportToAllClients()messages are detectable from web content actually. Have we ruled that out? (while not being enabled in ESR 68 it will be for mobile IIRC).I think we are good. LocalizeAndReportToAllClients ends up calling ConsoleUtils.cpp which seems it's only logging to console. I also tested a bit with the different error handlers in Service Workers and I could not get a localized message.
