###!!! ASSERTION: Principal mismatch. Not good

NEW
Unassigned

Status

()

9 years ago
7 years ago

People

(Reporter: WeirdAl, Unassigned)

Tracking

({sec-other})

Trunk
x86
Windows XP
sec-other
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse], URL)

Attachments

(2 attachments)

Comment hidden (empty)
(Reporter)

Comment 1

9 years ago
Sorry, my browser submitted the report early.

Steps to reproduce:
(1) Build debug XULRunner, or debug Firefox (use -app)
(2) hg clone
http://hg.mozdev.org/verbosio/file/7fc1345e79e5/experimental/template-edit
template-edit
(3) xulrunner template-edit/application.ini
(4) Click "openTestWindow" button.  A XUL wizard will appear.
(5) Hit the "Next" button 4 times.  You will reach the "Build primary user
interface" page.

Right about now you should be hitting the assertion many, many times.  This is
what I was seeing when I wrote bug 474695 comment 12.

I'll be happy to reduce this testcase if desired.
(Reporter)

Comment 2

9 years ago
###!!! ASSERTION: Principal mismatch.  Not good: '!aAllowShortCircuit || result == doGetObjectPrincipal(origObj, PR_FALSE)', file .../mozilla-1.9.1/caps/src/nsScriptSecurityManager.cpp, line 2381
A reduced testcase would help tremendously. If it's multi-file (given the binding) try zipping it and attaching it with MIME type application/java-archive.
Assignee: dveditz → nobody
(Reporter)

Comment 4

9 years ago
Created attachment 393410 [details]
minimized testcase application
(Reporter)

Comment 5

9 years ago
Created attachment 393411 [details]
stack trace

From the stack trace, it looks like it asserts trying to execute the binding's constructor.
2421        NS_ASSERTION(!aAllowShortCircuit ||
2422                     result == doGetObjectPrincipal(origObj, PR_FALSE),
2423                     "Principal mismatch.  Not good");
2424        
2425        return result;
(gdb) p result
$1 = (nsSystemPrincipal *) 0x847d90
(gdb) p doGetObjectPrincipal(origObj, 0)
$2 = (nsNullPrincipal *) 0x1bf78110

The key problem is here:

(gdb) frame 9
#9  0x00f9f322 in nsScriptSecurityManager::CheckFunctionAccess (this=0x83f9c0, aCx=0x11d3e00, aFunObj=0x14e0bc00, aTargetObj=0x14e0bb20) at /Users/bzbarsky/mozilla/vanilla/mozilla/caps/src/nsScriptSecurityManager.cpp:1677
(gdb) p JS_GetParent(aCx,aFunObj)
$24 = (JSObject *) 0x14e0bb20
(gdb) p JS_GetPrivate(aCx, $24)
$26 = (void *) 0x1bf7c0c0
(gdb) p (nsISupports*)$26
$27 = (XPCWrappedNative *) 0x1bf7c0c0
(gdb) p $27->mIdentity
$28 = (nsXULElement *) 0x1bf79fe0
$30 = (nsXULElement *) 0x1bf79fe0
(gdb) p $30->GetOwnerDoc()
$31 = (nsXULDocument *) 0x12d8200
(gdb) p $30->GetNodeParent()
$32 = (nsXULElement *) 0x827760
(gdb) p $30->GetNodeParent()->GetNodeParent()
$33 = (nsXULElement *) 0x827af0
(gdb) p $30->GetNodeParent()->GetNodeParent()->GetNodeParent()
$34 = (nsXULDocument *) 0x12d8200

But...

(gdb) p JS_GetParent(aCx, $24)
$35 = (JSObject *) 0x14e0b7c0
(gdb) p JS_GetParent(aCx, $35)
$36 = (JSObject *) 0x14e0b7a0
(gdb) p ((nsISupports*)JS_GetPrivate(aCx, $36))->mIdentity
$41 = (nsXMLDocument *) 0x10adc00

That last is what has the null principal.

So the issue is that the JS parent chain is not in sync with the node parent chain, which it should be for XUL elements.
And the relevant JS looks somewhat like this:

  var insertNode = getSingleNode(outputSource, {
    xmlns: XUL_NS
  });

  var outputRenderBox = document.getElementById("primaryUI-outputRenderBox");
  outputRenderBox.appendChild(insertNode);

where:

function getSingleNode(source, namespaceMap)
{
....
  const xml = openTag + source + "</root>";
  const doc = parser.parseFromString(xml, "application/xml");
  const node = doc.documentElement.firstChild;
  return (node.nextSibling ? null : node);

So there it makes sense that the node would have an XMLDocument as its grandparent.  Why are we not reparenting the node properly on that appendChild?
(Reporter)

Comment 8

9 years ago
Thanks, bz.  I filed this as security-sensitive based on other bugs I found in Bugzilla like this which were once classified.  Question:  should this still be security-sensitive?
Er, I thought I'd commented here with what's going on.  I wonder where that went...

The key is that we don't reparent wrappers for XUL nodes.  And since this code doesn't use new DOMParser but uses createInstance instead, it doesn't get a DOMParser with the right principal (which a web page would).

That said, a web page could trigger this assertion by simply having two separate documents with XUL nodes in them (same-origin, but different principals) and moving a node from one to the other.

I _think_ there isn't a security problem here, but could we throw in cases when moving a XUL node, not reparenting, and the two principals involved aren't equal, just in case?
(Reporter)

Comment 10

9 years ago
bz: I'm not sure I can simply move the node without reparenting and still hit the assertion.  For the XBL constructor to fire, the binding has to attach, and that requires the bound node be in the rendered document.

Unless I'm missing something obvious.
I have no idea what comment 10 is talking about....
[sg:nse] to match bug 474695.
Whiteboard: [sg:nse]

Comment 13

7 years ago
Opening up per comment 9 and a discussion with bz on irc.
Group: core-security
You need to log in before you can comment on or make changes to this bug.