577512 - (CVE-2010-3171) (more) cross-domain information leakage with Math.random()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Veditz [:dveditz]]

Attachment: reporter's paper (pdf)

Amit Klein of Trusteer reports that our previous fixes for bug 464071 and bug 475585 were insufficient and sends the attached paper.

He would like to know "within one (1) week from today, whether you confirm and acknowledge the technical nature of the vulnerabilities described in the attachment. Of course, I'm available for questions/discussions/suggestions at any time. If you need an extension, please let me know in advance."

He may be disclosing this issue in the future at "a security conference, in which case I would ask you to defer your own disclosure to a date as close to the conference as possible."

Abstract 
--------
While Mozilla attempted to address the issues of cross domain information leakage (through Math.random) in Firefox 3.6.4 (and above), Firefox 3.5.10 and Firefox 4.0 (alpha and beta), there is still a security vulnerability in the way the isolation is implemented, which enables cross domain leakage. In fact, it may make it easier to attack Firefox in some cases, compared to previous versions.

Additionally, a concerned is raised on the entropy provided in the seed to the Math.random PRNG, which may enable more powerful attacks.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Gah.  The patch in bug 475585 could really have used a DOM reviewer...

Shouldn't we reseed the rng at the point where we clear the outer window scope during page navigation?

Comment 2

[Andreas Gal :gal]

Yes, we should re-seed. And I also have very serious doubts about the severity of this attack. Math.random is a terrible PRNG, its completely underspecified and doesn't give you any guarantees what kind of randomness or entropy you get out of it. It shouldn't be used for anything but casual random number generation in the first place.

Comment 3

[Andreas Gal :gal]

bz, we currently have the rng state in cx, but really it should be in the scope actually (global object). I can clear the cx the clear scope request comes in on, but thats conceptually not guaranteed to be the right one (its in our embedding I am pretty sure though).

Comment 4

[Boris Zbarsky [:bzbarsky]]

> but really it should be in the scope actually 

Conceptually, yes.

If there is api to reseed the rng, we could just call that in the DOM code...

Comment 5

[Andreas Gal :gal]

Attachment: patch

Attached is a patch that re-seeds when the scope is cleared. Should we seed from /dev/random? I initialize libc's prng and seed with that, but stacking prngs is usually a bad idea.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 456472 [details] [diff] [review]
patch

Please make this compile on Windows.  As discussed, the seeding is a side-show; might not be worth worrying about.

Comment 7

[Andreas Gal :gal]

Attachment: patch

minimal fix

Comment 8

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/d5c78b20be2d

Comment 9

[Andreas Gal :gal]

I landed the patch on tracemonkey with a harmless commit message.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 456479 [details] [diff] [review]
patch

Presumably this simple patch applies to the branches, and if so please request approval. Otherwise please attach branch versions.

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d5c78b20be2d

Comment 12

[Daniel Veditz [:dveditz]]

Is there a reason this can't go into the next set of 1.9.2.x and 1.9.1.x releases?

Comment 13

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/cbc5fcbf9560

Comment 14

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d3ba41cd5a5d

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 456479 [details] [diff] [review]
patch

Requesting approval1.9.0.next on this patch as well, since Gavin informs me this is needed for bug 475585 to be useful.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

What is the STR here for reproducing the bug? I want to verify the fix.

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 456479 [details] [diff] [review]
patch

Approved for 1.9.0.20, a=dveditz for release-drivers

Comment 18

[Reed Loden [:reed]]

This bug was forgotten when writing the security advisories, most likely because it didn't have the proper flags set on the attachment. :(

Please be mindful of this in the future and ensure proper branch-landing procedure is always followed.