Isolate JS pseudorandom number generator state per compartment

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

({sec-low})

Other Branch
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(1 attachment)

Since the Math.random() algorithm is pretty trivial, the PRNG state is stored in JSContext, and contexts are shared across domains, a web page can theoretically observe how many times Math.random has been called in all other domains and even in chrome code.

This side channel is long-known, and it's not terribly sensitive information, but what the heck -- it's silly to be sharing this state, and super easy to fix.

(I believe Mark S. Miller proposed a change to ES6 to require each global to have its own PRNG stream. It isn't in the latest draft.)
Posted patch v1Splinter Review
Assignee: general → jorendorff
Attachment #690611 - Flags: review?(luke)
Comment on attachment 690611 [details] [diff] [review]
v1

Makes sense.  I would not be opposed if you renamed js_InitRandom to be a JSRuntime:: member of js:: non-member function.
Attachment #690611 - Flags: review?(luke) → review+
Whiteboard: [js:t]
This bounced because there's Windows-only code in js/src/assembler/jit randomly using random_next, whose signature this patch changes.

Trying again.
https://hg.mozilla.org/mozilla-central/rev/7702047b003f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Didn't we fix this twice already, in bug 475585 and bug 577512? :/
Keywords: sec-low
Those fixes were not the same as this one.  We didn't have anything like a compartment to store randomness in until recently, ish.  Those fixes just chose the next best thing.  Admittedly, the summaries/comments there probably didn't make that as clear as they could have.
You need to log in before you can comment on or make changes to this bug.