183223 - Address security issues for exposing JSObjects via IDispatch

Status: RESOLVED INVALID

(Core :: XPConnect, enhancement)

Description

[David Bradley]

The original goal of the IDispatch implementation was to allow JSObjects to be
viewed and manipulated via the IDispatch interface. Security concerns were
raised with allowing just any JSObject to be wrapped and passed into a COM
interface. To avoid this issue for now, wrapping arbitrary JSObjects has been
turned off.

I'm still not clear on the details of the security issues. In simple situations
we currently wrap arbitrary JSObjects and pass them to XPCOM objects. Is this
any different than doing it with COM. In this context, we're dealing with a
plugin and I'm not real familiar with the plugin framework. Would this bypass
some protection or insulation that the plugin framework provides?

Comment 1

[David Bradley]

Forgot to mention that when this is done, we should factor out the exception
logic between XPCDispatchTearOff::Invoke and nsXPCWrappedJSClass::CallMethod

Comment 2

[David Bradley]

So this bug is now back in the spotlight. And I'm trying to identify the
specific security concerns. If you had a scriptable plugin and JavaScript passed
it a JSObject to a method that took an XPCOM interface, it would wrap it. If it
happens to be a wrapped native, it's still going to go through all the normal
security checks to see that invoking the method or accessing the property is ok.

Comment 3

[Mitchell Stoltz (not reading bugmail)]

That sounds OK to me, but there's probably some nuance I'm missing. I'd like to
hear from jband about that.

Comment 4

[John Bandhauer]

I don't have any overriding concerns here. In general, for XPConnect we are
talking about well known interfaces with fixed methods described in idl. The
security system can control access in a granular way if it chooses. With
IDispatch  things are wide open - objects can dynamically change to expose
whatever methods and properties they want to expose. In the iteration of
dbradley's COMConnect that I reviewed he was using JS property enumeration to
automatically find and expose methods and properties. I commented that this was
counter to how xpconnect has always worked; i.e. rather than have the JS code
make an assertion about which properties and methods it wishes to expose (via
well defined interfaces) this code was just exposing whatever it could find. I'm
a few steps away from the requirements now (and I'm happy to let y'all make
whatever decision you'd like), but at the time I just wanted to make sure this
was a conscious decision and not an oversight with unintended consequences.

Comment 5

[David Bradley]

Thanks, jband, I had forgotten the details and needed a refresh.

So now the question is, is there any way to control this better. All I have is a
method that takes an IDispatch interface and a JS object that someone has
created. Could we require that our parent be of some known JS object. That would
at least prevent COM objects from being able to walk outside of the JSObject
passed in and converting other parts of it to IDispatch based pointers. Another
option would be to have an agreed upon property that would enumerate the desired
properties to reflect into IDispatch. The latter seems the best restrictive
approach, but could become combersome if there are a lot of properties. In our
current case, there's only three, but I don't know what we can expect from other
uses.

Comment 6

[David Bradley]

Moving out

Comment 7

[David Bradley]

Moving out again. Most embedding uses right now use the whitelist of a few
trusted controls so there's littel security risk. And the default Mozilla is to
have an empty white list.

Before any embedder would switch to a black list, this would need to be addressed.

Comment 8

[David Bradley]

Moving out

Comment 9

[Ed Morley [:emorley]]

The IDispatch API was removed in bug 662000.

-> INVALID

[Filter bugspam on idispatchinvalid]