296704 - trusted event reinitialization allows data-theft

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[shutdown]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: 

Mozilla has the special "trusted" flag on event objects, and uses it to
deny untrusted events to do some special operations. So untrusted
events raised by web pages cannot do clipboard copy, paste, etc...

However, malicious page can hijack trusted events by reinitializing
such events using DOM initEvent() family in the event handlers,
and can perform limited subset of the privileged operations.

Reproducible: Always

Steps to Reproduce:
1. open the testcase.
2. press "alt-a" key.
Actual Results:  
trusted events can be reinitialized by initEvent() family.

Expected Results:  
initEvent() family during dispatching-phase must be ignored.
So, trusted events cannot be hijacked by web pages.

http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-Event-initEvent
According to DOM Level2 Events Spec, initEvent() family
called during event dispatching-phase must be ignored.

Comment 1

[shutdown]

Attachment: testcase

testcase: clipboard text and local file can be stealed.

works on firefox 1.0.4 and firefox trunk 2005060423.
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050604 Firefox/1.0+

Comment 2

[Johnny Stenback (:jst)]

Fix coming up.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Prevent events from ever being initialized or dispatched more than once.

There may be a more efficient way to do this, with less code even. But I'm not
sure where that would be to catch all cases, so for now this is probably the
safest approach. Alternatively I guess we could move the code that marks events
as being dispatched already into nsEventListenerManager::HandleEvent() as I
*think* that all event handler invocations *always* go through there. And if no
handler is ever called, there's no *real* reason to mark the event since no
untrusted code could ever get a reference to the event (unless it was created
by script, in which case it'll be properly marked trustes/untrusted anyways).

Thoughts?

Comment 4

[Hixie (not reading bugmail)]

I don't see why you shouldn't be able to dispatch an event twice. If the event
is already being dispatched, then dispatching it again should raise
DISPATCH_REQUEST_ERR. Once an event has been passed to the dispatchEvent method,
 then calling initEventXXX() on it should do nothing.

Once a trusted event has finished being dispatched, its trusted flag should be
reset, so that if it is redispatched, it doesn't do any damage.

Beyond that I see no reason to stop redispatching or reinitialising.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 185875 [details] [diff] [review]
Prevent events from ever being initialized or dispatched more than once.

It's going to take me a day or two to get to this.... If I don't by the 19th,
though, then I won't be able to until July.

Comment 6

[Jay Patel [:jay]]

If bz can't get to this soon, can anyone else jump in here and get this fixed
for 1.0.5/1.7.9?

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 185875 [details] [diff] [review]
Prevent events from ever being initialized or dispatched more than once.

>-#define NS_PRIV_EVENT_UNTRUSTED_PERMITTED 0x0800
>+#define NS_EVENT_FLAG_DISPATCH_STARTED    0x0800
>+#define NS_PRIV_EVENT_UNTRUSTED_PERMITTED 0x1000

bump the untrusted define to 0x8000 so we don't move it around each time we add
a new flag. (Potentially breaks existing add-ons otherwise binary compatible
with unchanged interfaces.)

sr=dveditz

Comment 8

[Johnny Stenback (:jst)]

Attachment: Same as above but unmark the dispatch bit when we're done dispatching an event, and permit re-dispatching of DOM events (but reset trusted flags if the caller isn't trusted).

To be on the safe side, this makes is so that re-dispatching an event still
works as long as the event isn't in the process of being dispatched already.
This also makes sure that if an event is re-dispatched and the event is
trusted, it makes sure the caller is also trusted. If not, it unsets the
trusted bit when re-initializing the event. This code piggy-backs on the
NS_EVENT_FLAG_STOP_DISPATCH_IMMEDIATELY bit to know that an event has already
been dispatched, the bit gets set when we leave the DOM event loop, and it's
checked when dispatching an event.

diff -w coming up.

Comment 9

[Johnny Stenback (:jst)]

Attachment: diff -w of the above for review.

This should fix this bug while letting people reuse events in a safe way.

Comment 10

[Johnny Stenback (:jst)]

*** Bug 292943 has been marked as a duplicate of this bug. ***

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 186960 [details] [diff] [review]
diff -w of the above for review.

r=dveditz
a=dveditz for branches and trunk

Comment 12

[Johnny Stenback (:jst)]

Attachment: Aviary and mozilla1.7 branch version of the above

Comment 13

[Johnny Stenback (:jst)]

Fixed on trunk and aviary branches. There are some differences between 1.7 and
aviary branches that make this hard to apply there, need to figure that out
before landing on the 1.7 branch.

Comment 14

[Jay Patel [:jay]]

dveditz and jst need to discuss any discreprencies with their checkins on the
1.7 branch to get this patch checked in there.

Comment 15

[Chase Phillips]

Comment on attachment 186981 [details] [diff] [review]
Aviary and mozilla1.7 branch version of the above

Per 1.0.5 meeting, needs check-in by dveditz.

Comment 16

[Daniel Veditz [:dveditz]]

Checked into mozilla 1.7 branch for jst

Comment 17

[Boris Zbarsky [:bzbarsky]]

Don't early returns on various errors in HandleDOMEvent methods mean we won't
remove the "dispatching" flag from some events?

Comment 18

[Johnny Stenback (:jst)]

Attachment: Changes missing from the branches.

Comment 19

[Johnny Stenback (:jst)]

Attachment: Missed one part of the missing code.

This unsets the NS_EVENT_FLAG_STOP_DISPATCH_IMMEDIATELY bit when
re-initializing events to make them dispatchable again. Dveditz found this part
was missing.

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 188995 [details] [diff] [review]
Missed one part of the missing code.

r/sr=dveditz

Comment 21

[Johnny Stenback (:jst)]

Missing changes landed on the aviary *and* 1.7 branches now.

Comment 22

[Jay Patel [:jay]]

v.fixed on Aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9)
Gecko/20050711 Firefox/1.0.5 using attached testcase.  Alt-A no longer
compromises your clipboard.

Comment 23

[Daniel Veditz [:dveditz]]

Adding distributors

Comment 24

[Daniel Veditz [:dveditz]]

FF1.0.5 advisories published