Closed Bug 1382499 Opened 2 years ago Closed 2 years ago

Touch API leaks absolute screen coordinates

Categories

(Core :: DOM: Events, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: arthur, Assigned: cfu)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor 10286][fingerprinting][fp:m3])

Attachments

(3 files)

In Tor Browser we currently disable the Touch API, but we also have defense-in-depth patches to prevent absolute screen coordinates from being leaked to scripts. Here are the current patches:

https://torpat.ch/10286

We would like to propose uplifting these patches.
Priority: -- → P3
Whiteboard: [tor 10286] → [tor 10286][fingerprinting][fp:m3]
Assignee: nobody → cfu
Attachment #8891935 - Flags: review?(bzbarsky)
Attachment #8891935 - Flags: review?(arthuredelstein)
According to the Tor patch, I modified RadiusX, RadiusY, RotationAngle, Force, and the webidl with the same implementation as ScreenX, ScreenY.
However, they are not only accessed through webidl so there is not always a caller type.

For equality test,
- mozilla::dom::Touch::Equals
http://searchfox.org/mozilla-central/source/dom/events/Touch.cpp#187-190
I compare the members directly.

For other accesses,
- DispatchPointerFromMouseOrTouch
http://searchfox.org/mozilla-central/source/layout/base/PresShell.cpp#7101-7102
- mozilla::MultiTouchInput::MultiTouchInput
http://searchfox.org/mozilla-central/source/widget/InputData.cpp#133-136
I just pass CallerType::System.

Will this cause any problem?
Comment on attachment 8891935 [details]
Bug 1382499 - Enhance fingerprinting resistance for Touch API

https://reviewboard.mozilla.org/r/162970/#review168354

::: dom/events/Touch.cpp:167
(Diff revision 1)
>  
> +int32_t
> +Touch::RadiusX(CallerType aCallerType) const
> +{
> +  if (nsContentUtils::ResistFingerprinting(aCallerType)) {
> +    return 1;

Why 1 and not 0?  The spec seems to call out 0 as the "not known" value...

Someone familiar with the touch API should probably review this value.
Attachment #8891935 - Flags: review?(bzbarsky) → review+
We need your expertise to help us clarify if it is proper to use the value 1 for radiusX/radiusY in order to resist fingerprinting.
Thank you very much.
Flags: needinfo?(bugs)
0 should be used when radius isn't known.
https://w3c.github.io/touch-events/#dom-touch-radiusx
Flags: needinfo?(bugs)
Attachment #8893269 - Flags: review?(bugs)
Attachment #8893270 - Flags: review?(arthuredelstein)
Arthur also agrees with that it is more reasonable to spoof radiusX/radiusY with 0, so I updated the patch and changed the return values.

The test is ready as well.
Please note that in the original Tor patch, it compares touch.radiusX/radiusY to touchParams.rx/ry when privacy.resistFingerprinting = false.
In my local test, however, I always get different values.
For example, we have rx = 2 and ry = 3 in touchParams but in the Touch object, the values are radiusX = 4 and radiusY = 6.
It seems that AppUnitsPerDevPixel leads to this result.
So I guess it is machine-dependent and modified the test so that it only checks if radiusX and radiusY are correctly spoofed when privacy.resistFingerprinting = true.
Comment on attachment 8893269 [details]
Bug 1382499 - Fix typo in testing/mochitest/tests/SimpleTest/EventUtils.js

https://reviewboard.mozilla.org/r/164312/#review169898
Attachment #8893269 - Flags: review?(bugs) → review+
Comment on attachment 8891935 [details]
Bug 1382499 - Enhance fingerprinting resistance for Touch API

https://reviewboard.mozilla.org/r/162970/#review171930
Attachment #8891935 - Flags: review?(arthuredelstein) → review+
Comment on attachment 8893270 [details]
Bug 1382499 - Add test cases

https://reviewboard.mozilla.org/r/164314/#review171934
Attachment #8893270 - Flags: review?(arthuredelstein) → review+
Attachment #8893270 - Flags: review?(bugs)
Comment on attachment 8893270 [details]
Bug 1382499 - Add test cases

https://reviewboard.mozilla.org/r/164314/#review172240
Attachment #8893270 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1dcdc22d4919
Enhance fingerprinting resistance for Touch API r=arthuredelstein,bz
https://hg.mozilla.org/integration/autoland/rev/c25b50b0edc5
Fix typo in testing/mochitest/tests/SimpleTest/EventUtils.js r=smaug
https://hg.mozilla.org/integration/autoland/rev/0ebb9924affa
Add test cases r=arthuredelstein,smaug
Keywords: checkin-needed
Flags: needinfo?(cfu)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0175751964b5
Enhance fingerprinting resistance for Touch API r=arthuredelstein,bz
https://hg.mozilla.org/integration/autoland/rev/927e576d7971
Fix typo in testing/mochitest/tests/SimpleTest/EventUtils.js r=smaug
https://hg.mozilla.org/integration/autoland/rev/79d10d864775
Add test cases r=arthuredelstein,smaug
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.