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.
Since 18.104.22.168 will be the last FF1.5 it'd be nice to squash this one in it.
Created attachment 261180 [details] [diff] [review] Fix. This fixes this bug by adding the missing security check to nsEventReceiverSH::AddEventListenerHelper().
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.
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.
Comment on attachment 261194 [details] [diff] [review] With GET_PROPERTY check too. approved for 22.214.171.124 and 126.96.36.199, 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:188.8.131.52pre) Gecko/20070430 Firefox/184.108.40.206pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:220.127.116.11pre) Gecko/20070430 BonEcho/18.104.22.168pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070428 Minefield/3.0a5pre