Opened 18 months ago

Closed 3 weeks ago

#21787 closed task (fixed)

Make sure exposing the calendar information does not leak the locale

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

Description

Exposes a JS Intl API to get calendar information (https://bugzilla.mozilla.org/show_bug.cgi?id=1287503). We should make sure that this does not leak the locale or allows to narrow it down.

Child Tickets

Change History (15)

comment:1 Changed 17 months ago by gk

Keywords: tbb-7.0-must added

More tickets for 7.0.

comment:2 Changed 17 months ago by gk

Keywords: tbb-7.0-must-alpha added; tbb-7.0-must removed

Getting more tickets on our alpha radar.

comment:3 Changed 17 months ago by gk

Priority: MediumHigh

Moving the investigation tickets to higher priority.

comment:4 Changed 17 months ago by arthuredelstein

Keywords: ff59-esr TorBrowserTeam201705R added; ff52-esr removed
Status: newneeds_review

In ESR52, I confirmed in the code and by manual observation that this functionality is not exposed in content. The getCalendarInfo function can be used from chrome code like this:

const mozIntl = Components.classes["@mozilla.org/mozintl;1"]
                          .getService(Components.interfaces.mozIMozIntl);
let x = {};
mozIntl.addGetCalendarInfo(x);
let calendarInfo = x.getCalendarInfo();

The only place in ESR52 where this is done is in a test:
https://dxr.mozilla.org/mozilla-esr52/source/toolkit/components/mozintl/test/test_mozintl.js

But this is not possible from a content script. So I think we should postpone this ticket to ff59-esr.

comment:5 Changed 17 months ago by gk

Keywords: tbb-7.0-must-alpha TorBrowserTeam201705R removed
Status: needs_reviewassigned

Thanks, moving to esr59.

comment:6 Changed 8 months ago by gk

Keywords: ff60-esr added; ff59-esr removed

Firefox 60 is the new ESR.

comment:7 Changed 6 weeks ago by arthuredelstein

Keywords: TorBrowserTeam201808R added
Status: assignedneeds_review

This API remains chrome-only. I think there's no intention to expose it to content. So I would suggest closing this ticket.

comment:8 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam201808 added; TorBrowserTeam201808R removed
Status: needs_reviewneeds_revision

Replying to arthuredelstein:

This API remains chrome-only. I think there's no intention to expose it to content. So I would suggest closing this ticket.

Ugh, that took me quite some time... What do you mean with "chrome-only"? It seems content might be able to get a user to trigger the API via the <input> element, no? See https://blog.nightly.mozilla.org/2017/06/12/datetime-inputs-enabled-on-nightly/ for some examples: there is definitely a localization component that is exposed to content. Not sure if JS can made to access that directly but I bet that at least the resulting rendering differences might give a hint about a possible used locale. This is "no issue" for the Tor Browser alpha but only as our content policy hack breaks this feature. We are about to remove it, though.

See: https://bugzilla.mozilla.org/show_bug.cgi?id=1283384 and https://bugzilla.mozilla.org/show_bug.cgi?id=1329589 for some context.

comment:9 Changed 4 weeks ago by gk

Marked #26604 as a duplicate. Mark mentioned in #26604:

If necessary, we can set the dom.forms.datetime pref to false to remove support for these <input> types.

Additional relevant bugs:

https://bugzilla.mozilla.org/show_bug.cgi?id=1337319
https://bugzilla.mozilla.org/show_bug.cgi?id=1399036 (enabling the feature by default on all builds).

comment:10 Changed 4 weeks ago by arthuredelstein

Keywords: TorBrowserTeam201808R added; TorBrowserTeam201808 removed
Status: needs_revisionneeds_review

Unfortunately I had somehow missed the use of mozIntl.getCalendarInfo(...) in the date-time picker code. :(

Here's a patch for review. It uses the "en-US" locale for the date/time picker when privacy.spoof_english == 2.

https://github.com/arthuredelstein/tor-browser/commit/21787

comment:11 Changed 3 weeks ago by gk

Cc: mcs brade pospeselr added

Just one nit:

-        this.mResetButton.style.visibility = "hidden";
+          this.mResetButton.style.visibility = "hidden";

There is no need to indent that piece of the code. I tested the patch on a Linux box. Waiting on a second review either by mcs/brade or by pospeselr.

comment:12 in reply to:  11 Changed 3 weeks ago by arthuredelstein

Replying to gk:

Just one nit:

-        this.mResetButton.style.visibility = "hidden";
+          this.mResetButton.style.visibility = "hidden";

There is no need to indent that piece of the code. I tested the patch on a Linux box. Waiting on a second review either by mcs/brade or by pospeselr.

Thanks -- here a new version with that fixed:

https://github.com/arthuredelstein/tor-browser/commit/21787+1

comment:13 Changed 3 weeks ago by mcs

r=brade, r=mcs
This looks good to us and some quick testing shows that it works (at least for some examples on https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/date).

At first I was worried that some occurrences of mSpoofEnglish were missing this. in front of them, but I think the global JS object is this in those places so the code is OK.

comment:14 in reply to:  13 Changed 3 weeks ago by arthuredelstein

Replying to mcs:

r=brade, r=mcs
This looks good to us and some quick testing shows that it works (at least for some examples on https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/date).

At first I was worried that some occurrences of mSpoofEnglish were missing this. in front of them, but I think the global JS object is this in those places so the code is OK.

Thanks, good catch. I think it's better to make the this explicit. So I made a new version that uses this.mSpoofEnglish everywhere:

https://github.com/arthuredelstein/tor-browser/commit/21787+2

comment:15 Changed 3 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

Fixed with commit 7150252e7a0213aeddfe3c0aed294aae4cfcb3b8 on tor-browser-60.1.0esr-8.0-1, thanks everyone!

Note: See TracTickets for help on using tickets.