Page MenuHomePhabricator

mw.user.generateRandomSessionId not so random
Closed, ResolvedPublic

Description

We have been using generateRandomSessionId [1] to aid our research on sendBeacon support:

https://gerrit.wikimedia.org/r/#/c/175953/13/modules/ext.wikimediaEvents.sendBeacon.js

And found it to be not so "random".

We have about 56.000 records in which our client side generated id should be unique, but on those there are 29 duplicates. (Please note that this is nothing to do with sendBeacon itself as these numbers come from the control experiment that is contrasting "regular" event data with "send beacon" data)

We have looked into the browsers that are produccing duplicated data and for the most part is Safari in its many flavors (we can provide more detail data upon request).

Perhaps we should consider using: http://caniuse.com/#search=getRandomValues when supported?

We have enough "good" unique data to do our experiment so that is no issue, this is just an FYI.

[1] https://git.wikimedia.org/blob/mediawiki%2Fcore.git/ed646ee688057e6022a00e2965f3ca7095cc9ec8/resources%2Fsrc%2Fmediawiki%2Fmediawiki.user.js#L51

Event Timeline

Nuria raised the priority of this task from to Needs Triage.
Nuria updated the task description. (Show Details)
Nuria changed Security from none to None.
Aklapper triaged this task as Medium priority.Dec 15 2014, 1:05 PM

This is a more acute problem in mobile where Safari is much more prevalent (in our user base on enwiki at least)

gerritbot subscribed.

Change 187876 had a related patch set uploaded (by Nuria):
Using cryptoAPI if available in generateRandomSessionId

https://gerrit.wikimedia.org/r/187876

Patch-For-Review

Krinkle renamed this task from generateRandomSessionId on mediawiki.user.js not so random to mw.user.generateRandomSessionId not so random .Feb 14 2015, 10:26 AM
Krinkle closed this task as Resolved.
Krinkle removed a project: Patch-For-Review.
Krinkle subscribed.

Has anyone upstreamed this bug to Apple?

I believe this is a known issue and not per se a bug, Math.random is not supposed to be cryptographically strong, one of many reports on the subject: https://code.google.com/p/chromium/issues/detail?id=246054