[tbb-bugs] #22919 [Applications/Tor Browser]: Form tracking and OS fingerprinting (only Windows, but without Javascript)

[Tor Bug Tracker & Wiki]

#22919: Form tracking and OS fingerprinting (only Windows, but without Javascript)
-------------------------------------------------+-------------------------
 Reporter:  basvd                                |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Major                                |     Resolution:
 Keywords:  tbb-fingerprinting,                  |  Actual Points:
  TorBrowserTeam201910                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by acat):

 * status:  needs_review => needs_revision
 * keywords:  tbb-fingerprinting, TorBrowserTeam201910R => tbb-
     fingerprinting, TorBrowserTeam201910


Comment:

 Note that the original code is pushing ints:
 {{{
   mBoundary.AppendInt(rand());
   mBoundary.AppendInt(rand());
   mBoundary.AppendInt(rand());
 }}}

 but the patch is pushing bytes:
 {{{
   mBoundary.AppendInt(rand_buf[0]);
   mBoundary.AppendInt(rand_buf[1]);
   mBoundary.AppendInt(rand_buf[2]);
 }}}

 `rand()` returns an integer in the range `0..RAND_MAX`. I think `RAND_MAX`
 depends on the system, in glibc it's `2147483647`, in Windows is `32767`.
 But it's guaranteed to be at least `32767`.

 So the patch is making the boundary 24 bit long instead of the original
 48-96 bits (assuming rand() was actually seeded, which is a different
 problem). I think the patch should be amended to always provide 96 bits of
 entropy.

 FWIW: I've noticed the function [https://searchfox.org/mozilla-
 central/rev/97976753a21c1731e18177de9e5ce78ea3b3da2d/js/src/jsmath.cpp#492
 js::GenerateRandomSeed] which might be useful here. I think it uses the
 `RandomNum` that tom mentioned and has a fallback using `PRMJ_Now()` if
 that fails.

 So I think we could simply do:
 {{{
   mBoundary.AppendInt(js::GenerateRandomSeed());
   mBoundary.AppendInt(static_cast<uint32_t>(js::GenerateRandomSeed()));
 }}}

 and just `#include "jsmath.h"` for a 3 line patch. What do you think, tom?

 That's to preserve the original 96 bits of entropy, we could have 128 for
 the same price, but not sure whether changing the response sizes for this
 case is a good idea.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22919#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online