Closed Bug 820180 Opened 8 years ago Closed 8 years ago

Isolate JS pseudorandom number generator state per compartment


(Core :: JavaScript Engine, defect)

Other Branch
Not set





(Reporter: jorendorff, Assigned: jorendorff)


(Keywords: sec-low, Whiteboard: [js:t])


(1 file)

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.)
Attached patch v1Splinter Review
Assignee: general → jorendorff
Attachment #690611 - Flags: review?(luke)
Comment on attachment 690611 [details] [diff] [review]

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.
Closed: 8 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.