Last Comment Bug 440308 - XSS by using XMLHttpRequest and onreadystatechange handler
: XSS by using XMLHttpRequest and onreadystatechange handler
Status: VERIFIED FIXED
[sg:high]
: testcase, verified1.8.1.15, verified1.8.1.16
Product: Core
Classification: Components
Component: Security (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug] (high review load, please consider other reviewers)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-19 01:11 PDT by moz_bug_r_a4
Modified: 2008-08-25 02:15 PDT (History)
9 users (show)
samuel.sidler+old: blocking1.8.1.15+
dveditz: blocking1.8.1.16+
dveditz: wanted1.8.1.x+
asac: blocking1.8.0.next+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed patch (1.04 KB, patch)
2008-06-19 07:23 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
jst: review+
jst: superreview+
dveditz: approval1.8.1.15+
Details | Diff | Review

Description moz_bug_r_a4 2008-06-19 01:11:50 PDT
Please see bug 403168.

This is fx2-only.  On fx2, nsXMLHttpRequest::ChangeState() does not call
CheckInnerWindowCorrectness(), thus, it's possible to perform an XSS attack by
using onreadystatechange handler.

(On trunk, nsXMLHttpRequest::ChangeState() calls NotifyEventListeners(), which
calls CheckInnerWindowCorrectness().)
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2008-06-19 07:23:19 PDT
Created attachment 325759 [details] [diff] [review]
proposed patch
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2008-06-19 10:14:38 PDT
Comment on attachment 325759 [details] [diff] [review]
proposed patch

-      onReadyStateChangeListener) {
+      onReadyStateChangeListener &&
+      NS_SUCCEEDED(CheckInnerWindowCorrectness())) {

Looks good. This'll be the fourth caller of CheckInnerWindowCorrectness(), and it's inline. Probably worth un-inlining it now while you're here.
Comment 4 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2008-06-19 10:50:09 PDT
Well, is it really worth for the branch.
Comment 5 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2008-06-19 11:00:43 PDT
Comment on attachment 325759 [details] [diff] [review]
proposed patch

I'm not sure if this should go in to .15 or .16, but .15 is the only one I can ask approval for.
Comment 6 Daniel Veditz [:dveditz] 2008-06-20 11:18:33 PDT
Comment on attachment 325759 [details] [diff] [review]
proposed patch

Approved for 1.8.1.15 and 1.8.1.16, a=dveditz for release-drivers

Please land on both branches (MOZILLA_1_8_BRANCH for 1.8.1.16 and GECKO181_20080612_RELBRANCH for 1.8.1.15) and give the bug both fixed1.8.1.15 and fixed1.8.1.16 keywords
Comment 7 Daniel Veditz [:dveditz] 2008-06-20 14:31:26 PDT
Fix checked into both 1.8 branch and _relbranch
Comment 8 Samuel Sidler (old account; do not CC) 2008-06-21 10:45:24 PDT
Resolving this bug as FIXED since it's branch-only.
Comment 9 Al Billings [:abillings] 2008-06-23 11:31:02 PDT
Verified this with the new 2.0.0.15 build (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.15) Gecko/2008062305 Firefox/2.0.0.15) and reproduced the bug on the same machine with shipped 2.0.0.14.
Comment 10 Al Billings [:abillings] 2008-07-07 16:55:13 PDT
Verified this with 2.0.0.16 Firefox as well (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.16) Gecko/2008070205 Firefox/2.0.0.16).
Comment 11 Samuel Sidler (old account; do not CC) 2008-08-25 02:15:25 PDT
Comment on attachment 325759 [details] [diff] [review]
proposed patch

This patch had approval for 1.8.1.16, but apparently the flags got moved out. Clearing that flag to clear the queries.

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