Last Comment Bug 577512 - (CVE-2010-3171) (more) cross-domain information leakage with Math.random()
(CVE-2010-3171)
: (more) cross-domain information leakage with Math.random()
Status: RESOLVED FIXED
[sg:low], fixed-in-tracemonkey [qa-ne...
: privacy
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Andreas Gal :gal
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-08 10:13 PDT by Daniel Veditz [:dveditz]
Modified: 2015-10-16 11:47 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
betaN+
wanted
.9+
.9-fixed
.12+
.12-fixed


Attachments
reporter's paper (pdf) (95.57 KB, application/pdf)
2010-07-08 10:13 PDT, Daniel Veditz [:dveditz]
no flags Details
patch (2.22 KB, patch)
2010-07-08 11:10 PDT, Andreas Gal :gal
bzbarsky: review-
Details | Diff | Review
patch (598 bytes, patch)
2010-07-08 12:25 PDT, Andreas Gal :gal
bzbarsky: review+
dveditz: approval1.9.0.next+
Details | Diff | Review

Description Daniel Veditz [:dveditz] 2010-07-08 10:13:57 PDT
Created attachment 456463 [details]
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 [:bz] 2010-07-08 10:20:07 PDT
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 2010-07-08 10:57:00 PDT
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 2010-07-08 10:58:41 PDT
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 [:bz] 2010-07-08 11:06:07 PDT
> 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 2010-07-08 11:10:39 PDT
Created attachment 456472 [details] [diff] [review]
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 [:bz] 2010-07-08 11:44:32 PDT
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 2010-07-08 12:25:28 PDT
Created attachment 456479 [details] [diff] [review]
patch

minimal fix
Comment 8 Andreas Gal :gal 2010-07-08 16:01:41 PDT
http://hg.mozilla.org/tracemonkey/rev/d5c78b20be2d
Comment 9 Andreas Gal :gal 2010-07-08 16:05:02 PDT
I landed the patch on tracemonkey with a harmless commit message.
Comment 10 Daniel Veditz [:dveditz] 2010-07-16 10:16:57 PDT
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 12 Daniel Veditz [:dveditz] 2010-07-22 19:25:33 PDT
Is there a reason this can't go into the next set of 1.9.2.x and 1.9.1.x releases?
Comment 15 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-08-16 19:40:54 PDT
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] 2010-08-17 15:56:20 PDT
What is the STR here for reproducing the bug? I want to verify the fix.
Comment 17 Daniel Veditz [:dveditz] 2010-09-01 12:24:13 PDT
Comment on attachment 456479 [details] [diff] [review]
patch

Approved for 1.9.0.20, a=dveditz for release-drivers
Comment 18 Reed Loden [:reed] (use needinfo?) 2010-09-08 00:13:08 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.