Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Bug 577512 (CVE-2010-3171)

(more) cross-domain information leakage with Math.random()

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: dveditz, Assigned: gal)

Tracking

({privacy})

Trunk
privacy
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+, status2.0 wanted, blocking1.9.2 .9+, status1.9.2 .9-fixed, blocking1.9.1 .12+, status1.9.1 .12-fixed)

Details

(Whiteboard: [sg:low], fixed-in-tracemonkey [qa-needs-str])

Attachments

(2 attachments, 1 obsolete attachment)

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

7 years ago
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?
Assignee: nobody → general
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status1.9.1: --- → ?
status1.9.2: --- → ?
status2.0: --- → ?
Component: General → JavaScript Engine
Keywords: privacy
QA Contact: general → general
Whiteboard: [sg:low]
(Assignee)

Comment 2

7 years ago
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.
(Assignee)

Comment 3

7 years ago
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

7 years ago
> 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...
(Assignee)

Comment 5

7 years ago
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.
Assignee: general → gal
(Assignee)

Updated

7 years ago
Attachment #456472 - Flags: review?(bzbarsky)

Comment 6

7 years ago
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.
Attachment #456472 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 7

7 years ago
Created attachment 456479 [details] [diff] [review]
patch

minimal fix
Attachment #456472 - Attachment is obsolete: true
Attachment #456479 - Flags: review?(bzbarsky)

Updated

7 years ago
Attachment #456479 - Flags: review?(bzbarsky) → review+

Updated

7 years ago
Summary: (more) cross-domain information leakage → (more) cross-domain information leakage with Math.random()
(Assignee)

Comment 8

7 years ago
http://hg.mozilla.org/tracemonkey/rev/d5c78b20be2d
Whiteboard: [sg:low] → [sg:low], fixed-in-tracemonkey
(Assignee)

Comment 9

7 years ago
I landed the patch on tracemonkey with a harmless commit message.
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
status1.9.1: ? → wanted
status1.9.2: ? → wanted
status2.0: ? → wanted
Keywords: testcase-wanted
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

7 years ago
http://hg.mozilla.org/mozilla-central/rev/d5c78b20be2d
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Is there a reason this can't go into the next set of 1.9.2.x and 1.9.1.x releases?
blocking1.9.1: needed → ?
blocking1.9.2: needed → ?
blocking1.9.1: ? → .12+
blocking1.9.2: ? → .9+

Updated

7 years ago
blocking2.0: ? → betaN+

Comment 13

7 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/cbc5fcbf9560
status1.9.1: wanted → .12-fixed

Comment 14

7 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d3ba41cd5a5d
status1.9.2: wanted → .9-fixed
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.
Attachment #456479 - Flags: approval1.9.0.next?
What is the STR here for reproducing the bug? I want to verify the fix.
Whiteboard: [sg:low], fixed-in-tracemonkey → [sg:low], fixed-in-tracemonkey [qa-needs-str]
Comment on attachment 456479 [details] [diff] [review]
patch

Approved for 1.9.0.20, a=dveditz for release-drivers
Attachment #456479 - Flags: approval1.9.0.next? → approval1.9.0.next+
Attachment #456479 - Flags: approval1.9.2.9?
Attachment #456479 - Flags: approval1.9.1.12?
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.
Alias: CVE-2010-3171
Attachment #456479 - Flags: approval1.9.2.9?
Attachment #456479 - Flags: approval1.9.1.12?
Attachment #456463 - Attachment description: reporer's paper (pdf) → reporter's paper (pdf)
Group: core-security
Keywords: testcase-wanted
You need to log in before you can comment on or make changes to this bug.