Opened 7 weeks ago

Last modified 2 weeks ago

#30683 needs_revision defect

Properties in dom/locales/$lang/chrome/ allow detecting user locale

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-fingerprinting-locale, TorBrowserTeam201907
Cc: mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

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

Child Tickets

Change History (9)

comment:1 in reply to:  description Changed 5 weeks ago by gk

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

Last edited 5 weeks ago by gk (previous) (diff)

comment:2 Changed 4 weeks ago by acat

Keywords: TorBrowserTeam201906R added
Status: newneeds_review

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-source:* pages, which should not be accessible from web content.

Do we want all these tbb-fingerprinting-locale fixed 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.

comment:3 in reply to:  2 Changed 4 weeks ago by gk

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-source:* pages, which should not be accessible from web content.

Do we want all these tbb-fingerprinting-locale fixed 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).

comment:4 Changed 3 weeks ago by gk

fwiw: mik317 reminded us to check for error message because of corrupted png images, too (which is available in MediaDocument.properties), thanks!

comment:5 Changed 2 weeks ago by gk

Keywords: TorBrowserTeam201907R added; TorBrowserTeam201906R removed

No reviews in June 2019 anymore, moving them.

comment:6 Changed 2 weeks ago by gk

Cc: mcs brade added
Keywords: TorBrowserTeam201907 added; TorBrowserTeam201907R removed
Status: needs_reviewneeds_revision

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-US strings will always be available in, say fr, builds that don't come with fr lang packs.

mcs, brade could you have a second look?

comment:7 Changed 2 weeks ago by acat

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.

comment:8 in reply to:  6 Changed 2 weeks ago by mcs

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-US strings will always be available in, say fr, builds that don't come with fr lang 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.

comment:9 Changed 2 weeks ago by cypherpunks

hard-coded a bunch of strings

Never do that.

Note: See TracTickets for help on using tickets.