286629 - Allow untrusted script access to Components.lookupMethod

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[(not currently active) Ted Mielczarek]

Components.lookupMethod allows script to get the "original" implementation of a
method.  In XBL that is bound to an untrusted DOM, this method is not available,
so webpages can break XBL bindings by redefining necessary methods such as
createElement.

This can be seen in particular in the Flashblock extension, which attaches an
XBL binding to embed tags, and uses document.createElement, which means
arbitrary pages can redefine that function and break the binding, thus letting
Flash through.

Comment 1

[Johnny Stenback (:jst)]

Attachment: Let anyone call Components.lookupMethod

This change lets anyone call Components.lookupMethod() from anywhere. I can't
see any harm in permitting that, so I'm proposing we check this in.

Comment 2

[Daniel Veditz [:dveditz]]

Comment on attachment 177799 [details] [diff] [review]
Let anyone call Components.lookupMethod

r=dveditz

If we're going to make changes here, how 'bout allowing isSuccessCode too? No
reason to block it, and could prevent errors in code that doesn't expect a
non-zero success.

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 177799 [details] [diff] [review]
Let anyone call Components.lookupMethod

>+    static const char* allowed[] = { "lookupMethod", nsnull};

Uber-nit: space after nsnull before the closing brace?

I agree with Dan, why not expose isSuccessCode too?  Gotta stick up for
NS_COMFALSE (blech! ;-) here.

/be

Comment 4

[Benjamin Smedberg]

Do we still perform the correct security checks at some point? (I'm not up on
whether we do the method security checks at invocation or reference).

Comment 5

[Johnny Stenback (:jst)]

(In reply to comment #4)
> Do we still perform the correct security checks at some point? (I'm not up on
> whether we do the method security checks at invocation or reference).

The security check happens in XPCWrappedNative::CallMethod(), which is invoked
no matter how you get at a JS function for a XPConnect implemented
method/getter/setter on a native wrapper, so it's all good AFAIKT (and testing
verified this too).

Comment 6

[Johnny Stenback (:jst)]

Attachment: Diff with the suggested changes.

Comment 7

[Johnny Stenback (:jst)]

Attachment: Testcase showing we still do the security checks.

Comment 8

[chris hofmann]

Comment on attachment 177799 [details] [diff] [review]
Let anyone call Components.lookupMethod

a=chofmann

Comment 9

[Johnny Stenback (:jst)]

FIXED.