An attacker can add an event listener to a target site. This allows an attacker to perform an XSS attack as well as to sniff keystrokes (like bug 18553). When a JS code calls addEventListener method, the native addEventListener method is called through nsEventReceiverSH::AddEventListenerHelper unless the JS code uses Components.lookupMethod to get addEventListener method. And, nsEventReceiverSH::AddEventListenerHelper does not check whether caller can access a given event target. Thus, an attacker can add an event listener to a target site. w = open("target site"); document.addEventListener.call(w, "keypress", x, true); And, when an event handler registered by the attacker is called, caller's context is the target site, and the attacker can control a string conversion of an event object. Thus, the attacker can perform an XSS attack.
Assignee: dveditz → general
Component: Security → DOM
OS: Windows XP → All
QA Contact: toolkit → ian
Hardware: PC → All
Since 126.96.36.199 will be the last FF1.5 it'd be nice to squash this one in it.
Assignee: general → jst
Created attachment 261180 [details] [diff] [review] Fix. This fixes this bug by adding the missing security check to nsEventReceiverSH::AddEventListenerHelper().
Attachment #261180 - Flags: review?(mrbkap) → review+
Comment on attachment 261180 [details] [diff] [review] Fix. This looks pretty good, but should we also do an ACCESS_CALL_METHOD check here? sr=bzbarsky with that added or an explanation of why it's not needed.
Attachment #261180 - Flags: superreview?(bzbarsky) → superreview+
bz, I could do a CALL_METHOD check in stead the GET_PROPERTY check, but I don't immediately see the need to do both checks. Sound good to you, or am I overlooking something something here?
They're equivalent modulo CAPS policies, but those could screw with this, I think. Does XPConnect do both checks if this were not overridden via classinfo? If so, that seems like the safe thing to do.
XPConnect would do both checks, I'll add the GET_PROPERTY check as well.
Created attachment 261194 [details] [diff] [review] With GET_PROPERTY check too. Same as above, with another check added to check that the caller can get the property addEventListener as well as call that method.
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Comment on attachment 261194 [details] [diff] [review] With GET_PROPERTY check too. approved for 188.8.131.52 and 184.108.40.206, a=dveditz for release-drivers. Please wait to land this until much nearer code-freeze date since the patch makes it fairly obvious where to look for the problem.
Verified on both branches and trunk using the following builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:220.127.116.11pre) Gecko/20070430 Firefox/18.104.22.168pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:22.214.171.124pre) Gecko/20070430 BonEcho/126.96.36.199pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070428 Minefield/3.0a5pre
Status: RESOLVED → VERIFIED
Keywords: fixed188.8.131.52, fixed184.108.40.206 → verified220.127.116.11, verified18.104.22.168
You need to log in before you can comment on or make changes to this bug.