Opened 5 years ago

Closed 5 years ago

#13026 closed defect (fixed)

Verify screenX and screenY are spoofed sanely

Reported by: mikeperry Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: ff31-esr, tbb-easy, tbb-testcase, tbb-fingerprinting, TorBrowserTeam201409Easy
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In Firefox 28, window.screenX and window.screenY were changed to report CSS pixels instead of device pixels. We should ensure we're still properly reporting content window resolution here.

Child Tickets

Change History (7)

comment:1 Changed 5 years ago by mikeperry

Keywords: tbb-easy tbb-testcase added

This is probably easy to test with FF31 and some JS, and/or an existing site like http://browserspy.dk/screen.php.

comment:2 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201409Easy added; TorBrowserTeam201409 removed

comment:3 Changed 5 years ago by gacar

We already had a test for screenX and screenY:
https://gitweb.torproject.org/boklm/tor-browser-bundle-testsuite.git/blob/HEAD:/selenium-tests/test_fp_screen_coords.py#l11

Do we need to test any additional behavior or just need to pass this test with ESR31 based TBB? I'd be happy to write a new test if former is the case.

comment:4 Changed 5 years ago by gacar

Checking the relevant Mozilla bug #943668 and landed FF patch, it seems existing TBB patch #0021 is still ok but needs to be updated for ESR31 to cover this new method: nsGlobalWindow::GetScreenXY

For the device/CSS pixel difference, TBB bluntly returns 0 from the methods changed in the Firefox's patch (GetScreenX, GetScreenY). So I guess it should be ok.

comment:5 in reply to:  4 ; Changed 5 years ago by mcs

Replying to gacar:

Checking the relevant Mozilla bug #943668 and landed FF patch, it seems existing TBB patch #0021 is still ok but needs to be updated for ESR31 to cover this new method: nsGlobalWindow::GetScreenXY

It looks like Arthur already patched that method:
https://github.com/arthuredelstein/tor-browser/commit/0c95ccb313bfc059e8d3433edeb6c4b4a9309569

For the device/CSS pixel difference, TBB bluntly returns 0 from the methods changed in the Firefox's patch (GetScreenX, GetScreenY). So I guess it should be ok.

If Arthur agrees that he has already taken care of the new methods, this bug should be closed.

comment:6 in reply to:  5 Changed 5 years ago by arthuredelstein

Replying to mcs:

Replying to gacar:

Checking the relevant Mozilla bug #943668 and landed FF patch, it seems existing TBB patch #0021 is still ok but needs to be updated for ESR31 to cover this new method: nsGlobalWindow::GetScreenXY

It looks like Arthur already patched that method:
https://github.com/arthuredelstein/tor-browser/commit/0c95ccb313bfc059e8d3433edeb6c4b4a9309569

For the device/CSS pixel difference, TBB bluntly returns 0 from the methods changed in the Firefox's patch (GetScreenX, GetScreenY). So I guess it should be ok.

If Arthur agrees that he has already taken care of the new methods, this bug should be closed.

I think this patch is OK, but it would be good if someone other than me can review that patch for correctness :). Note that there is a fixup for this patch (https://github.com/arthuredelstein/tor-browser/commit/5d00edbfe6492c7636bc349450878d2a7f5b54fa) and also some mochitest-plain regression tests (https://github.com/arthuredelstein/tor-browser/commit/07ef3362124e1df51f02782220c3519ab69f4cfd), which currently pass.

comment:7 Changed 5 years ago by mikeperry

Resolution: fixed
Status: newclosed

I just tested the nightly on browserspy, and looked at these patches. Everything looks good.

Note: See TracTickets for help on using tickets.