Last Comment Bug 640339 - (CVE-2011-1202) generate-id() function leaks information about valid heap addresses
(CVE-2011-1202)
: generate-id() function leaks information about valid heap addresses
Status: RESOLVED FIXED
[sg:low]
:
Product: Core
Classification: Components
Component: XSLT (show other bugs)
: unspecified
: x86 Windows 7
: -- normal (vote)
: ---
Assigned To: Jonas Sicking (:sicking) PTO Until July 5th
:
Mentors:
https://cevans-app.appspot.com/static...
: 651571 (view as bug list)
Depends on: 1243337
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-09 14:28 PST by Daniel Veditz [:dveditz]
Modified: 2016-02-19 10:07 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
Macaw+
.1-fixed
.17-fixed
.19-fixed


Attachments
patch to fix (4.41 KB, patch)
2011-03-09 16:22 PST, Jonas Sicking (:sicking) PTO Until July 5th
peterv: review+
dveditz: approval2.0+
dveditz: approval1.9.2.17+
dveditz: approval1.9.1.19+
Details | Diff | Review
patch security (902.96 KB, patch)
2011-04-30 19:11 PDT, tarak5825@gmail.com
no flags Details | Diff | Review

Description Daniel Veditz [:dveditz] 2011-03-09 14:28:42 PST
As demonstrated at the test URL and announced on Chris Evans' blog the XPath generate-id() function returns a valid heap address which might provide a useful handle in other attacks. Appears to affect all browsers one way or another (Chrome was patched before announcing this).

http://scarybeastsecurity.blogspot.com/2011/03/multi-browser-heap-address-leak-in-xslt.html
Comment 1 Jonas Sicking (:sicking) PTO Until July 5th 2011-03-09 16:22:03 PST
Created attachment 518223 [details] [diff] [review]
patch to fix

Needs a bit more testing, but I think this should do it. I originally used the address of the txExecutionState itself, but since that usually lives on the stack it's possible that that'll be on a predictable address.
Comment 2 Peter Van der Beken [:peterv] 2011-03-10 07:42:22 PST
Comment on attachment 518223 [details] [diff] [review]
patch to fix

>diff --git a/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp b/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp

>+    PRUword nodeid = ((PRUword)aNode.mNode) - ((PRUword)aBase.mNode);

Don't think you need all those brackets.

>diff --git a/content/xslt/src/xslt/txGenerateIdFunctionCall.cpp b/content/xslt/src/xslt/txGenerateIdFunctionCall.cpp

>+            "called xslt extension function \"current\" with wrong context");

s/current/generate-id/

I don't think this can leak info about adresses anymore.
Comment 3 Jonas Sicking (:sicking) PTO Until July 5th 2011-03-11 19:24:58 PST
Dan, I don't know how you want to do about landing this on branches given that I don't think it can land on trunk right now.
Comment 4 Daniel Veditz [:dveditz] 2011-03-14 10:44:22 PDT
Comment on attachment 518223 [details] [diff] [review]
patch to fix

Approved for 1.9.2.16 and 1.9.1.18, a=dveditz for release-drivers
Comment 5 Daniel Veditz [:dveditz] 2011-03-14 10:45:26 PDT
S'ok, we'll take the branches now and 4.0.1 when we can.
Comment 6 Jonas Sicking (:sicking) PTO Until July 5th 2011-03-15 09:56:09 PDT
Checked in to branches:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/768b54fa2f7d
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a201c429788b

Leaving open as it hasn't been check in to trunk yet.

Also nominating for the 2.0 branch as that'll likely be a separate landing.
Comment 7 Daniel Veditz [:dveditz] 2011-04-11 18:23:38 PDT
Comment on attachment 518223 [details] [diff] [review]
patch to fix

Approved for the mozilla2.0 repository, a=dveditz for release-drivers
Comment 8 Jonas Sicking (:sicking) PTO Until July 5th 2011-04-11 23:32:57 PDT
Checked in on m-c and 2.0:

http://hg.mozilla.org/mozilla-central/rev/c2bd5cf4070e
http://hg.mozilla.org/releases/mozilla-2.0/rev/e01dc3fc20d3
Comment 9 Peter Van der Beken [:peterv] 2011-04-20 12:12:51 PDT
*** Bug 651571 has been marked as a duplicate of this bug. ***
Comment 10 tarak5825@gmail.com 2011-04-30 19:11:32 PDT
Created attachment 529326 [details] [diff] [review]
patch security
Comment 11 Jonas Sicking (:sicking) PTO Until July 5th 2011-04-30 19:19:04 PDT
Tarak, what is this attachment supposed to be? It looks like an executable.
Comment 12 Jonas Sicking (:sicking) PTO Until July 5th 2011-04-30 19:19:33 PDT
Comment on attachment 529326 [details] [diff] [review]
patch security

Marking obsolete for now as I suspect this was added by mistake.

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