62069 - Remote XUL widgets busted

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P3)

Description

[David Hyatt]

Trees and menus are busted in remote XUL, and I am now blocked in my XBL form
control work by the fact that I cannot call QueryInterface.  I need to (from
within a Web page or remote XUL file) be able to do the following

var menuBox = boxObject.QueryInterface(Components.interfaces.nsIMenuBoxObject);

I assume that the problem is Components.interfaces?  I get the error message:

"Access to XPConnect service denied."

Comment 1

[David Hyatt]

The object has implemented nsISecurityCheckedComponent, but I guess that isn't
being used for whatever is causing the error.

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Did this problem just show up or has this been broken for some time? I don't
know of any recent security policy changes that would have caused this. Was
remote access to Components.interfaces by untrusted code ever allowed?

I will soon have to put a more DOM-like access control system on XPConnect,
which will make nsISecurityCheckedComponent unnecessary.
SecurityCheckedComponent was a poorly conceived security mechanism which was
created to fix a couple of specific problems but will not scale well.

Hyatt, if you need a quick fix, please post a testcase for this behavior and
I'll track down where exatly you're getting denied.

Comment 3

[John Bandhauer]

netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect"); ?

As you figured out nsISecurityCheckedComponent only impacts calls made to the
object implementing that interface. Should more of the stuff hanging off
Components be available w/o security restrictions? By default it is all off
limits. 'Components.interfaces' will throw an exception. Should it not? Is the
list of interfaces available on the machine worth protecting? The list of
classes? If we exp[ose those objects, which methods and properties on those
objects do we also expose? Part of the reason for not showing them is that they
were assumed to be of no use. Another part was as an additional security
stumbling block to unanticipated exploits.

When Norris was the security czar he simply wanted to avoid the whole problem
(and focus on all the other stuff that needed doing) so we got a very binary
security model for xpconnect. Is it time to work out something better? Who's
signing up?

Comment 4

[David Hyatt]

Ok, I've verified that nsXPCComponents (which is presumably the Components
object in JS) doesn't implement nsISecurityCheckedComponent, and so you're not
allowed to access Components from remote pages or remote XUL.  This is the
source of the problem.

I believe Components should implement this interface and allow the retrieval of
the "interfaces" property by anyone.  We could then make interfaces implement
nsISecurityCheckedComponent and then perhaps it could only allow the retrieval
of "blessed" interfaces that we could designate.

Comment 5

[David Hyatt]

I'll sign up to do it, if my approach sounds valid.  Any suggestions for how to
designate certain IIDs as blessed?

Comment 6

[John Bandhauer]

Dave. I can just crank out a patch for you if you want.

I'm more concerned with the work of devising and implementing a coherent plan 
for the whole of xpconnect security within the browser.

nsXPCComponents_Interfaces does its work using the jsapi and uses no security 
checks for getting its properties (the specific interface objects). Once you can 
get Components.interfaces you can get at any of them. Of course, the idea is 
that you can't do much with them. If it's OK with mstoltz, then we can just 
expose the whole mess of them.

Comment 7

[John Bandhauer]

Attachment: proposed fix

Comment 8

[David Hyatt]

I think it's ok.  All our IIDs are easily obtainable by looking at our source
anyway.  They're not secure or privileged.  I could presumably look up the raw
IID I wanted and use it in place of Components.interfaces.nsIFoo anyway.

Yes, please, whip up a patch.  Thanks, John!

Comment 9

[John Bandhauer]

try the patch. Given....

<script>
try{
    var foo = Components.interfaces.nsIJSIID;
    if(foo)
        document.write("got it"+"<br>");
} catch(e){
    document.write("threw 1<br>");
}

document.write("<p>");

try{
    document.write("Components.interfaces.nsIJSIID = "+
                    Components.interfaces.nsIJSIID +"<br>");
} catch(e){
    document.write("threw 2<br>");
}

document.write("<p>");

try{
    for(n in Components.interfaces)
        document.write(n+"<br>");
} catch(e){
    document.write("threw 3<br>");
}

document.write("<p>");

try{
    for(n in Components.classes)
        document.write(n+"<br>");

} catch(e){
    document.write("threw 4<br>");
}

document.write("<p>");

</script>

...it will succeed on the first block, throw on the second, succeed on the 
third, and throw on the fourth. I think this ought to do what you need.

Comment 10

[David Hyatt]

I can't get the patch to apply.  Darnit.  Tried waterson's POSIXLY_CORRECT stuff
and all that jazz.  The patch still fails.  Could you just mail me the two files?

Thanks!

Comment 11

[David Hyatt]

Never mind.  I was successful after repeated clubbings of the patch file.

Comment 12

[David Hyatt]

Worked like a charm and fixed remote XUL!

sr=hyatt

Comment 13

[Brendan Eich [:brendan]]

r=[e-mail]

/be

Comment 14

[John Bandhauer]

fix checked in

Comment 15

[cls]

Just FYI, this checkin introduced an additional intermodule dependency for
xpconnect.

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/976178040.6659.gz

Comment 16

[John Bandhauer]

cls: Can you help here? Is it just a matter of adding a relative include path to 
the Makefile.in? or what? I'm not going to be around to do anything on this 
until this evening. Thanks.

Comment 17

[Brendan Eich [:brendan]]

Does caps export its generated headers before xpconnect?  It appears not to be,
and that makes sense.  I don't think a -I that "reaches around" in the source
tree is right at all -- xpconnect should not assume caps is part of its cvs
workarea, it should depend only on dist stuff.

Can we move nsISecurityCheckedComponent.idl to a more primitive place, perhaps
to xpcom/components?

/be

Comment 18

[cls]

The fix is to add caps to REQUIRES, which I've just checked in.

Comment 19

[John Bandhauer]

Awesome. Thanks. Will this blow up in cases where caps is not found (and not
actually used in xpconnect due to #ifdefs) - i.e. XPCONNECT_STANDALONE - or will
it fail silently?

Comment 20

[cls]

Yes, the changes will fail when building standalone.  They will need to be
properly ifdef'd.  There's another possible fix that I'll bring up in the
newsgroups as it deals with standalone modules in general.