Bug 911864 - Make in-content XBL event handlers allowuntrusted=false by default. r=smaug
authorBobby Holley <bobbyholley@gmail.com>
Fri, 01 Nov 2013 15:31:58 +0100
changeset 167635 fa28da03e7d9fde3c52e9c6d64f53bb1a65875e2
parent 167634 2662a96f83ef7aa73a4ed44476f86a8a284d8244
child 167636 c4f51fe7d6379196464dcb91b8123d0647361408
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs911864
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 911864 - Make in-content XBL event handlers allowuntrusted=false by default. r=smaug
content/xbl/src/nsXBLBinding.cpp
content/xbl/src/nsXBLBinding.h
content/xbl/src/nsXBLEventHandler.cpp
content/xbl/src/nsXBLEventHandler.h
content/xbl/test/file_bug821850.xhtml
content/xbl/test/test_bug403162.xhtml
content/xbl/test/test_bug639338.xhtml
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/xpcprivate.h
js/xpconnect/src/xpcpublic.h
--- a/content/xbl/src/nsXBLBinding.cpp
+++ b/content/xbl/src/nsXBLBinding.cpp
@@ -149,18 +149,19 @@ nsXBLService::getClass(nsCStringKey *k)
 {
   return static_cast<nsXBLJSClass*>(nsXBLService::gClassTable->Get(k));
 }
 
 // Implementation /////////////////////////////////////////////////////////////////
 
 // Constructors/Destructors
 nsXBLBinding::nsXBLBinding(nsXBLPrototypeBinding* aBinding)
-  : mMarkedForDeath(false),
-    mPrototypeBinding(aBinding)
+  : mMarkedForDeath(false)
+  , mUsingXBLScope(false)
+  , mPrototypeBinding(aBinding)
 {
   NS_ASSERTION(mPrototypeBinding, "Must have a prototype binding!");
   // Grab a ref to the document info so the prototype binding won't die
   NS_ADDREF(mPrototypeBinding->XBLDocumentInfo());
 }
 
 
 nsXBLBinding::~nsXBLBinding(void)
@@ -292,16 +293,30 @@ nsXBLBinding::UninstallAnonymousContent(
 }
 
 void
 nsXBLBinding::SetBoundElement(nsIContent* aElement)
 {
   mBoundElement = aElement;
   if (mNextBinding)
     mNextBinding->SetBoundElement(aElement);
+
+  if (!mBoundElement) {
+    return;
+  }
+
+  // Compute whether we're using an XBL scope.
+  //
+  // We disable XBL scopes for remote XUL, where we care about compat more
+  // than security. So we need to know whether we're using an XBL scope so that
+  // we can decide what to do about untrusted events when "allowuntrusted"
+  // is not given in the handler declaration.
+  nsCOMPtr<nsIGlobalObject> go = mBoundElement->OwnerDoc()->GetScopeObject();
+  NS_ENSURE_TRUE_VOID(go && go->GetGlobalJSObject());
+  mUsingXBLScope = xpc::UseXBLScope(js::GetObjectCompartment(go->GetGlobalJSObject()));
 }
 
 bool
 nsXBLBinding::HasStyleSheets() const
 {
   // Find out if we need to re-resolve style.  We'll need to do this
   // if we have additional stylesheets in our binding document.
   if (mPrototypeBinding->HasStyleSheets())
@@ -525,32 +540,33 @@ nsXBLBinding::InstallEventHandlers()
           if ((curr->GetType() & (NS_HANDLER_TYPE_XBL_COMMAND |
                                   NS_HANDLER_TYPE_SYSTEM)) &&
               (isChromeBinding || mBoundElement->IsInNativeAnonymousSubtree())) {
             flags.mInSystemGroup = true;
           }
 
           bool hasAllowUntrustedAttr = curr->HasAllowUntrustedAttr();
           if ((hasAllowUntrustedAttr && curr->AllowUntrustedEvents()) ||
-              (!hasAllowUntrustedAttr && !isChromeDoc)) {
+              (!hasAllowUntrustedAttr && !isChromeDoc && !mUsingXBLScope)) {
             flags.mAllowUntrustedEvents = true;
           }
 
           manager->AddEventListenerByType(handler,
                                           nsDependentAtomString(eventAtom),
                                           flags);
         }
       }
 
       const nsCOMArray<nsXBLKeyEventHandler>* keyHandlers =
         mPrototypeBinding->GetKeyEventHandlers();
       int32_t i;
       for (i = 0; i < keyHandlers->Count(); ++i) {
         nsXBLKeyEventHandler* handler = keyHandlers->ObjectAt(i);
         handler->SetIsBoundToChrome(isChromeDoc);
+        handler->SetUsingXBLScope(mUsingXBLScope);
 
         nsAutoString type;
         handler->GetEventName(type);
 
         // If this is a command, add it in the system event group, otherwise 
         // add it to the standard event group.
 
         // Figure out if we're using capturing or not.
--- a/content/xbl/src/nsXBLBinding.h
+++ b/content/xbl/src/nsXBLBinding.h
@@ -157,16 +157,17 @@ public:
   // Returns a live node list that iterates over the anonymous nodes generated
   // by this binding.
   nsAnonymousContentList* GetAnonymousNodeList();
 
 // MEMBER VARIABLES
 protected:
 
   bool mMarkedForDeath;
+  bool mUsingXBLScope;
 
   nsXBLPrototypeBinding* mPrototypeBinding; // Weak, but we're holding a ref to the docinfo
   nsCOMPtr<nsIContent> mContent; // Strong. Our anonymous content stays around with us.
   nsRefPtr<nsXBLBinding> mNextBinding; // Strong. The derived binding owns the base class bindings.
   nsRefPtr<nsXBLJSClass> mJSClass; // Strong. The class object also holds a strong reference,
                                    // which might be somewhat redundant, but be safe to avoid
                                    // worrying about edge cases.
 
--- a/content/xbl/src/nsXBLEventHandler.cpp
+++ b/content/xbl/src/nsXBLEventHandler.cpp
@@ -65,17 +65,18 @@ nsXBLMouseEventHandler::EventMatched(nsI
   return mouse && mProtoHandler->MouseEventMatched(mouse);
 }
 
 nsXBLKeyEventHandler::nsXBLKeyEventHandler(nsIAtom* aEventType, uint8_t aPhase,
                                            uint8_t aType)
   : mEventType(aEventType),
     mPhase(aPhase),
     mType(aType),
-    mIsBoundToChrome(false)
+    mIsBoundToChrome(false),
+    mUsingXBLScope(false)
 {
 }
 
 nsXBLKeyEventHandler::~nsXBLKeyEventHandler()
 {
 }
 
 NS_IMPL_ISUPPORTS1(nsXBLKeyEventHandler, nsIDOMEventListener)
@@ -91,17 +92,17 @@ nsXBLKeyEventHandler::ExecuteMatchedHand
   nsCOMPtr<EventTarget> target = aKeyEvent->InternalDOMEvent()->GetCurrentTarget();
 
   bool executed = false;
   for (uint32_t i = 0; i < mProtoHandlers.Length(); ++i) {
     nsXBLPrototypeHandler* handler = mProtoHandlers[i];
     bool hasAllowUntrustedAttr = handler->HasAllowUntrustedAttr();
     if ((trustedEvent ||
         (hasAllowUntrustedAttr && handler->AllowUntrustedEvents()) ||
-        (!hasAllowUntrustedAttr && !mIsBoundToChrome)) &&
+        (!hasAllowUntrustedAttr && !mIsBoundToChrome && !mUsingXBLScope)) &&
         handler->KeyEventMatched(aKeyEvent, aCharCode, aIgnoreShiftKey)) {
       handler->ExecuteHandler(target, aKeyEvent);
       executed = true;
     }
   }
   return executed;
 }
 
--- a/content/xbl/src/nsXBLEventHandler.h
+++ b/content/xbl/src/nsXBLEventHandler.h
@@ -80,26 +80,33 @@ public:
   {
     return mType;
   }
 
   void SetIsBoundToChrome(bool aIsBoundToChrome)
   {
     mIsBoundToChrome = aIsBoundToChrome;
   }
+
+  void SetUsingXBLScope(bool aUsingXBLScope)
+  {
+    mUsingXBLScope = aUsingXBLScope;
+  }
+
 private:
   nsXBLKeyEventHandler();
   bool ExecuteMatchedHandlers(nsIDOMKeyEvent* aEvent, uint32_t aCharCode,
                                 bool aIgnoreShiftKey);
 
   nsTArray<nsXBLPrototypeHandler*> mProtoHandlers;
   nsCOMPtr<nsIAtom> mEventType;
   uint8_t mPhase;
   uint8_t mType;
   bool mIsBoundToChrome;
+  bool mUsingXBLScope;
 };
 
 nsresult
 NS_NewXBLEventHandler(nsXBLPrototypeHandler* aHandler,
                       nsIAtom* aEventType,
                       nsXBLEventHandler** aResult);
 
 nsresult
--- a/content/xbl/test/file_bug821850.xhtml
+++ b/content/xbl/test/file_bug821850.xhtml
@@ -98,17 +98,20 @@ https://bugzilla.mozilla.org/show_bug.cg
           </body>
         </method>
         <property name="prop" exposeToUntrustedContent="true">
           <getter>return this._prop;</getter>
           <setter>this._prop = "set:" + val;</setter>
         </property>
       </implementation>
       <handlers>
-        <handler event="testevent" action="ok(true, 'called event handler'); finish();"/>
+        <handler event="testevent" action="ok(true, 'called event handler'); finish();" allowuntrusted="true"/>
+        <handler event="testtrusted" action="ok(true, 'called trusted handler'); window.wrappedJSObject.triggeredTrustedHandler = true;"/>
+        <handler event="keyup" action="ok(true, 'called untrusted key handler'); window.wrappedJSObject.triggeredUntrustedKeyHandler = true;" allowuntrusted="true"/>
+        <handler event="keydown" action="ok(true, 'called trusted key handler'); window.wrappedJSObject.triggeredTrustedKeyHandler = true;"/>
       </handlers>
     </binding>
   </bindings>
   <script type="application/javascript">
   <![CDATA[
 
   ok = parent.ok;
   is = parent.is;
@@ -201,16 +204,68 @@ https://bugzilla.mozilla.org/show_bug.cg
 
     // Tamper with the derived object. This doesn't affect the XBL scope thanks
     // to Xrays.
     bound.method = function() { return "heh"; };
     Object.defineProperty(bound, 'method', {value: function() { return "hah" }});
     Object.defineProperty(bound, 'prop', {value: "redefined"});
     bound.primitiveField = 321;
 
+    // We need a chrome window to create trusted events. This isn't really doable
+    // in child processes, so let's just skip if that's the case.
+    if (SpecialPowers.isMainProcess()) {
+      var Ci = SpecialPowers.Ci;
+      var chromeWin = SpecialPowers.wrap(window.top)
+                                   .QueryInterface(Ci.nsIInterfaceRequestor)
+                                   .getInterface(Ci.nsIWebNavigation)
+                                   .QueryInterface(Ci.nsIDocShell)
+                                   .chromeEventHandler.ownerDocument.defaultView;
+
+      // Untrusted events should not trigger event handlers without
+      // exposeToUntrustedContent=true.
+      window.triggeredTrustedHandler = false;
+      var untrustedEvent = new CustomEvent('testtrusted');
+      ok(!untrustedEvent.isTrusted, "Created an untrusted event");
+      bound.dispatchEvent(untrustedEvent);
+      ok(!window.triggeredTrustedHandler, "untrusted events should not trigger trusted handler");
+      var trustedEvent = new chromeWin.CustomEvent('testtrusted');
+      ok(trustedEvent.isTrusted, "Created a trusted event");
+      SpecialPowers.wrap(bound).dispatchEvent(trustedEvent);
+      ok(window.triggeredTrustedHandler, "trusted events should trigger trusted handler");
+
+      //
+      // We check key events as well, since they're implemented differently.
+      //
+      // NB: We don't check isTrusted on the events we create here, because
+      // according to smaug, old-style event initialization doesn't mark the
+      // event as trusted until it's dispatched.
+      //
+
+      window.triggeredUntrustedKeyHandler = false;
+      window.triggeredTrustedKeyHandler = false;
+
+      // Untrusted event, permissive handler.
+      var untrustedKeyEvent = document.createEvent('KeyboardEvent');
+      untrustedKeyEvent.initEvent('keyup', true, true);
+      bound.dispatchEvent(untrustedKeyEvent);
+      ok(window.triggeredUntrustedKeyHandler, "untrusted key events should trigger untrusted handler");
+
+      // Untrusted event, strict handler.
+      var fakeTrustedKeyEvent = document.createEvent('KeyboardEvent');
+      fakeTrustedKeyEvent.initEvent('keydown', true, true);
+      bound.dispatchEvent(fakeTrustedKeyEvent);
+      ok(!window.triggeredTrustedKeyHandler, "untrusted key events should not trigger trusted handler");
+
+      // Trusted event, strict handler.
+      var trustedKeyEvent = chromeWin.document.createEvent('KeyboardEvent');
+      trustedKeyEvent.initEvent('keydown', true, true);
+      SpecialPowers.wrap(bound).dispatchEvent(trustedKeyEvent);
+      ok(window.triggeredTrustedKeyHandler, "trusted key events should trigger trusted handler");
+    }
+
     // Hand control back to the XBL scope by dispatching an event on the bound element.
     bound.dispatchEvent(new CustomEvent('testevent'));
   }
 
   function checkThrows(fn) {
     try { fn(); ok(false, "Should have thrown"); }
     catch (e) { ok(!!/denied|insecure/.exec(e), "Should have thrown security exception: " + e); }
   }
--- a/content/xbl/test/test_bug403162.xhtml
+++ b/content/xbl/test/test_bug403162.xhtml
@@ -4,17 +4,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 -->
 <head>
   <title>Test for Bug 403162</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
   <bindings xmlns="http://www.mozilla.org/xbl">
     <binding id="test">
       <handlers>
-        <handler event="foo" action="XPCNativeWrapper.unwrap(window).triggerCount++"/>
+        <handler event="foo" action="XPCNativeWrapper.unwrap(window).triggerCount++" allowuntrusted="true"/>
       </handlers>
     </binding>
   </bindings>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=403162">Mozilla Bug 403162</a>
 <p id="display" style="-moz-binding: url(#test)"></p>
 <div id="content" style="display: none">
--- a/content/xbl/test/test_bug639338.xhtml
+++ b/content/xbl/test/test_bug639338.xhtml
@@ -4,19 +4,19 @@ https://bugzilla.mozilla.org/show_bug.cg
 -->
 <head>
   <title>Test for Bug 639338</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
   <bindings xmlns="http://www.mozilla.org/xbl">
     <binding id="test">
       <handlers>
-        <handler event="DOMMouseScroll" action="XPCNativeWrapper.unwrap(window).triggerCount++"/>
-        <handler event="DOMMouseScroll" modifiers="shift" action="XPCNativeWrapper.unwrap(window).shiftCount++"/>
-        <handler event="DOMMouseScroll" modifiers="control" action="XPCNativeWrapper.unwrap(window).controlCount++"/>
+        <handler event="DOMMouseScroll" action="XPCNativeWrapper.unwrap(window).triggerCount++" allowuntrusted="true"/>
+        <handler event="DOMMouseScroll" modifiers="shift" action="XPCNativeWrapper.unwrap(window).shiftCount++" allowuntrusted="true"/>
+        <handler event="DOMMouseScroll" modifiers="control" action="XPCNativeWrapper.unwrap(window).controlCount++" allowuntrusted="true"/>
       </handlers>
     </binding>
   </bindings>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=639338">Mozilla Bug 639338</a>
 <p id="display" style="-moz-binding: url(#test)"></p>
 <div id="content" style="display: none">
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -293,16 +293,23 @@ JSObject *GetXBLScope(JSContext *cx, JSO
     return scope;
 }
 
 bool AllowXBLScope(JSCompartment *c)
 {
   XPCWrappedNativeScope *scope = EnsureCompartmentPrivate(c)->scope;
   return scope && scope->AllowXBLScope();
 }
+
+bool UseXBLScope(JSCompartment *c)
+{
+  XPCWrappedNativeScope *scope = EnsureCompartmentPrivate(c)->scope;
+  return scope && scope->UseXBLScope();
+}
+
 } /* namespace xpc */
 
 XPCWrappedNativeScope::~XPCWrappedNativeScope()
 {
     MOZ_COUNT_DTOR(XPCWrappedNativeScope);
     DEBUG_TrackDeleteScope(this);
 
     // We can do additional cleanup assertions here...
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1364,16 +1364,17 @@ public:
     JSObject *EnsureXBLScope(JSContext *cx);
 
     XPCWrappedNativeScope(JSContext *cx, JS::HandleObject aGlobal);
 
     nsAutoPtr<JSObject2JSObjectMap> mWaiverWrapperMap;
 
     bool IsXBLScope() { return mIsXBLScope; }
     bool AllowXBLScope();
+    bool UseXBLScope() { return mUseXBLScope; }
 
 protected:
     virtual ~XPCWrappedNativeScope();
 
     static void KillDyingScopes();
 
     XPCWrappedNativeScope(); // not implemented
 
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -49,16 +49,23 @@ TransplantObject(JSContext *cx, JS::Hand
 JSObject *
 GetXBLScope(JSContext *cx, JSObject *contentScope);
 
 // Returns whether XBL scopes have been explicitly disabled for code running
 // in this compartment. See the comment around mAllowXBLScope.
 bool
 AllowXBLScope(JSCompartment *c);
 
+// Returns whether we will use an XBL scope for this compartment. This is
+// semantically equivalent to comparing global != GetXBLScope(global), but it
+// does not have the side-effect of eagerly creating the XBL scope if it does
+// not already exist.
+bool
+UseXBLScope(JSCompartment *c);
+
 bool
 IsSandboxPrototypeProxy(JSObject *obj);
 
 bool
 IsReflector(JSObject *obj);
 
 bool
 IsXrayWrapper(JSObject *obj);