Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#20981 closed defect (fixed)

The localized date string on Windows reveals local time

Reported by: gk Owned by: arthuredelstein
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-fingerprinting, tbb-regression, tbb-6.0-issues, TorBrowserTeam201701R
Cc: arthuredelstein, brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

https://blog.torproject.org/blog/tor-browser-602-released#comment-191587 mentioned it first and https://blog.torproject.org/blog/tor-browser-65a6-released#comment-225152 pointed it out again:

On Windows the localized Date string reveals the local timezone (again). I reproduced that with an en-US 6.5a6 bundle on a Windows 7 machine. On my Linux box it is not an issue.

Child Tickets

Change History (13)

comment:1 Changed 10 months ago by gk

Keywords: tbb-regression-6.0 added
Severity: NormalMajor

This regressed in Tor Browser 6.0. While the To Locale Format and To Locale String row showed the same string (e.g. "Thursday, December 15, 2016 10:00:00") in Tor Browser < 6.0 starting with 6.0 To Locale String differs AND shows the local time: "12/15/2016, Your-Local-Time".

Arthur can you take a look at it?

comment:2 Changed 10 months ago by gk

Keywords: tbb-6.0-regression added; tbb-regression-6.0 removed

comment:3 Changed 10 months ago by gk

Keywords: tbb-regression tbb-6.0-issues added; tbb-6.0-regression removed

I wonder whether #15299 would have caught this...

comment:4 Changed 10 months ago by arthuredelstein

Owner: changed from tbb-team to arthuredelstein
Status: newaccepted

comment:5 Changed 10 months ago by mcs

Cc: brade mcs added

comment:6 Changed 9 months ago by gk

Keywords: TorBrowserTeam201701 added; TorBrowserTeam201612 removed

Moving our tickets to January 2017

comment:7 Changed 9 months ago by cypherpunks

http://browserspy.dk/date.php on Linux:

  • the fields get filled only on low and medium-low security.
  • "Time and Date" is UTC, unlike my system's time, but my hardware clock is also UTC.
  • "Locale string" is always date +"%x, %r" in "en_US" format, unlike my system-wide setting.
  • "Difference between server and PC time" is consistent across two Linux systems on the same machine, with +/-2 second variation. The difference was noticeably reduced after running ntpdate pool.ntp.org and refreshing the page.
Last edited 9 months ago by cypherpunks (previous) (diff)

comment:8 Changed 9 months ago by arthuredelstein

Keywords: TorBrowserTeam201701R added; TorBrowserTeam201701 removed
Status: acceptedneeds_review

Here's a patch for review, tested on Windows:
https://github.com/arthuredelstein/tor-browser/commit/20981+1

comment:9 Changed 9 months ago by mcs

r=mcs

I did not test the patch, but it looks plausible.
And I guess it is okay to return malloc'd memory since that is done in other cases. Your patch might be a couple of lines shorter if you used uprv_strdup(), but that is not a big deal.

comment:10 in reply to:  9 ; Changed 9 months ago by arthuredelstein

Replying to mcs:

r=mcs

I did not test the patch, but it looks plausible.
And I guess it is okay to return malloc'd memory since that is done in other cases. Your patch might be a couple of lines shorter if you used uprv_strdup(), but that is not a big deal.

Thanks for reviewing it. That's a good suggestion. I've written a new patch using uprv_strdup and I will post it here later today once I am done testing.

I believe the malloc'd memory is necessary -- later code attempts to free it if I recall correctly.

comment:11 Changed 9 months ago by arthuredelstein

Here's my new patch. I'm having trouble building Firefox on Windows at the moment (nothing to do with the patch), so I haven't had a chance to test it.

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

comment:12 in reply to:  10 Changed 9 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to arthuredelstein:

I believe the malloc'd memory is necessary -- later code attempts to free it if I recall correctly.

Yes, in timezone.cpp:

#if U_PLATFORM_USES_ONLY_WIN32_API
    // hostID points to a heap-allocated location on Windows.
    uprv_free(const_cast<char *>(hostID));
#endif

Looks good to me. I tested the patch on a Windows machine and the issue is fixed with it.

Applied to tor-browser-45.6.0esr-6.5-1 (commit 025f08062d4e633432e0248bbd047ac5d61c4399). I guess this can make it into 6.5 as the patch is quite simple.

comment:13 Changed 9 months ago by arthuredelstein

Keywords: tbb-fingerprinting added; tbb-fingerinting removed
Note: See TracTickets for help on using tickets.