Bug 911864 - Make in-content XBL event handlers allowuntrusted=false by default. r=smaug, a=bajaj
authorBobby Holley <bobbyholley@gmail.com>
Fri, 01 Nov 2013 15:31:58 +0100
changeset 161306 9e1c21d8987c4879999c25a676ff06c7a8d642de
parent 161305 48545fba1c3c1e357b2a993250a60d9f10be333e
child 161307 eb748b48040248535ee9f1e877d202837522a609
push id4600
push userryanvm@gmail.com
push dateTue, 12 Nov 2013 17:09:20 +0000
treeherdermozilla-aurora@9e1c21d8987c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, bajaj
bugs911864
milestone27.0a2
Bug 911864 - Make in-content XBL event handlers allowuntrusted=false by default. r=smaug, a=bajaj
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);