Fixed event handlers
authorRiccardo Pelizzi <r.pelizzi@gmail.com>
Thu, 31 May 2012 01:01:45 -0400
changeset 46 8d32acd293a345bd95d0c839ec01ade37da4eda5
parent 45 c0fed27360961eadb57b6a1a36ffff6988a1e603
child 47 37105ba0f3916270d67887c3825a543c9c2aaeba
push id27
push userr.pelizzi@gmail.com
push dateThu, 31 May 2012 05:03:17 +0000
Fixed event handlers
xssfilter
--- a/xssfilter
+++ b/xssfilter
@@ -1,11 +1,11 @@
 # HG changeset patch
 # User Riccardo Pelizzi <r.pelizzi@gmail.com>
-# Parent 79262a88881d66b474921d6944e7ca9f45facf6a
+# Parent 78852a6d11abb1402be20ef77691a45bc34b724f
 Bug 528661 - Reflected XSS Filter for Firefox
 
 diff --git a/b2g/chrome/content/netError.xhtml b/b2g/chrome/content/netError.xhtml
 --- a/b2g/chrome/content/netError.xhtml
 +++ b/b2g/chrome/content/netError.xhtml
 @@ -161,16 +161,23 @@
          }
  
@@ -491,17 +491,17 @@ diff --git a/caps/src/nsScriptSecurityMa
 +    nsresult rv;
 +    nsIPrincipal* subjectPrincipal = ssm->GetSubjectPrincipal(cx, &rv);
 +
 +    NS_ASSERTION(NS_SUCCEEDED(rv),
 +                 "XSS: Failed to get nsIPrincipal from js context");
 +
 +    if (!subjectPrincipal) {
 +        // See bug 553448 for discussion of this case.
-+        NS_ASSERTION(!JS_GetSecurityCallbacks(cx)->findObjectPrincipals,
++        NS_ASSERTION(!JS_GetSecurityCallbacks(js::GetRuntime(cx))->findObjectPrincipals,
 +                     "XSS: Should have been able to find subject principal."
 +                     "Reluctantly allowing script.");
 +        return true;
 +    }
 +
 +    nsRefPtr<nsXSSFilter> xss;
 +    rv = subjectPrincipal->GetXSSFilter(getter_AddRefs(xss));
 +    // don't do anything unless there's a filter
@@ -4290,17 +4290,17 @@ new file mode 100644
 +   NONE, ATTACK, DONE],
 +  [GET, "hdr wrong", "outside",
 +   '<script>parent.postMessage("xss", "*");</script>', "0; disabled;",
 +   NONE, SAFE, DONE],
 +  [GET, "junk", "outside",
 +   '<script>parent.postMessage("xss", "*");</script>', "a0",
 +   NONE, ATTACK, DONE],
 +
-+]
++];
 +
 +function setXss (state){
 +  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
 +  var prefService = Components.classes["@mozilla.org/preferences-service;1"]
 +    .getService(Components.interfaces.nsIPrefService);
 +  var cspBranch = prefService.getBranch("security.xssfilter.");
 +  cspBranch.setBoolPref("enabled", state);
 +}
@@ -4821,241 +4821,82 @@ diff --git a/content/events/src/nsEventL
  }
  
  nsEventListenerManager::~nsEventListenerManager() 
  {
    // If your code fails this assertion, a possible reason is that
    // a class did not call our Disconnect() manually. Note that
    // this class can have Disconnect called in one of two ways:
    // if it is part of a cycle, then in Unlink() (such a cycle
-@@ -212,16 +224,19 @@ nsEventListenerManager::AddEventListener
- 
-   ls = mListeners.AppendElement();
-   ls->mListener = aListener;
-   ls->mEventType = aType;
-   ls->mTypeAtom = aTypeAtom;
-   ls->mWrappedJS = false;
-   ls->mFlags = aFlags;
-   ls->mHandlerIsString = false;
-+  ls->mXSSHandlerWasString = false;
-+  ls->mXSSChecked = false;
-+  ls->mXSSResult = true;
- 
-   nsCOMPtr<nsIXPConnectWrappedJS> wjs = do_QueryInterface(aListener);
-   if (wjs) {
-     ls->mWrappedJS = true;
-   }
-   if (aFlags & NS_EVENT_FLAG_SYSTEM_EVENT) {
-     mMayHaveSystemGroupListeners = true;
-   }
-@@ -489,16 +504,19 @@ nsEventListenerManager::SetJSEventListen
+@@ -489,16 +501,17 @@ nsEventListenerManager::SetJSEventListen
      }
    } else {
      ls->GetJSListener()->SetHandler(aHandler);
    }
  
    if (NS_SUCCEEDED(rv) && ls) {
      // Set flag to indicate possible need for compilation later
      ls->mHandlerIsString = !aHandler;
-+    // we only care about handlers that were strings at some point in time.
-+    ls->mXSSHandlerWasString |= !aHandler;
 +
      if (aPermitUntrustedEvents) {
        ls->mFlags |= NS_PRIV_EVENT_UNTRUSTED_PERMITTED;
      }
  
      *aListenerStruct = ls;
    } else {
      *aListenerStruct = nsnull;
    }
-@@ -591,16 +609,34 @@ nsEventListenerManager::AddScriptEventLi
+@@ -591,16 +604,26 @@ nsEventListenerManager::AddScriptEventLi
          scriptSample.AppendLiteral(" element");
          csp->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT,
                                   NS_ConvertUTF8toUTF16(asciiSpec),
                                   scriptSample,
                                   nsnull);
          return NS_OK;
        }
      }
-+
-+    // NOTE: if it's onload, we need to check now because it does not
-+    // go through handleevent later. why?
-+    // if (aName == nsGkAtoms::onload) {
-+    //   nsRefPtr<nsXSSFilter> xss;
-+    //   rv = doc->NodePrincipal()->GetXSSFilter(getter_AddRefs(xss));
-+    //   NS_ENSURE_SUCCESS(rv, rv);
-+
-+    //   if (xss) {
-+    //     PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+    //            ("XSS:Onload:check"));
-+    //     if (!xss->PermitsEventListener(aBody)) {
-+    //       PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+    //              ("XSS:Onload:blocked onload event"));
-+    //       return NS_ERROR_FAILURE;
-+    //     }
-+    //   }
-+    // }
++    nsRefPtr<nsXSSFilter> xss;
++    rv = doc->NodePrincipal()->GetXSSFilter(getter_AddRefs(xss));
++    NS_ENSURE_SUCCESS(rv, rv);
++    if (xss) {
++      if (!xss->PermitsEventListener(aBody)) {
++        PR_LOG(gXssPRLog, PR_LOG_DEBUG,
++               ("XSS:Onload:blocked onload event"));
++        return NS_OK;
++      }
++    }
    }
  
    // This might be the first reference to this language in the global
    // We must init the language before we attempt to fetch its context.
    if (NS_FAILED(global->EnsureScriptEnvironment())) {
      NS_WARNING("Failed to setup script environment for this language");
      // but fall through and let the inevitable failure below handle it.
    }
-@@ -800,24 +836,103 @@ nsEventListenerManager::HandleEventSubTy
+@@ -800,20 +823,18 @@ nsEventListenerManager::HandleEventSubTy
      result = CompileEventHandlerInternal(aListenerStruct,
                                           jslistener->GetEventContext() !=
                                             aPusher->GetCurrentScriptContext(),
                                           nsnull);
    }
  
    if (NS_SUCCEEDED(result)) {
      nsAutoMicroTask mt;
 -    // nsIDOMEvent::currentTarget is set in nsEventDispatcher.
--    result = aListener->HandleEvent(aDOMEvent);
-+    if (IsEventXSS(aListenerStruct, aCurrentTarget)) {
-+      result = NS_ERROR_FAILURE;
-+    } else {
-+      // nsIDOMEvent::currentTarget is set in nsEventDispatcher.
-+      result = aListener->HandleEvent(aDOMEvent);
-+    }
-+
+     result = aListener->HandleEvent(aDOMEvent);
    }
- 
+-
    return result;
  }
  
  /**
-+ * lazy check with the xss filter.
-+ */
-+bool
-+nsEventListenerManager::IsEventXSS(nsListenerStruct *aListenerStruct,
-+                                   nsPIDOMEventTarget* aCurrentTarget)
-+{
-+  // only makes sense to check string-based events
-+  if (!aListenerStruct->mXSSHandlerWasString) {
-+    return false;
-+  }
-+  // recycle result
-+  if (aListenerStruct->mXSSChecked) {
-+    PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+           ("CheckEventForXSS:has previous result"));
-+    return aListenerStruct->mXSSResult;
-+  }
-+
-+  // 1. xss filter
-+  nsIDocument* doc = nsnull;
-+  nsCOMPtr<nsINode> node = do_QueryInterface(aCurrentTarget);
-+  if (!node) {
-+    PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+           ("CheckEventForXSS:no node"));
-+    //nsCOMPtr<nsIWindow> win = do_QueryInterface(aCurrentTarget);
-+    return false;
-+  }
-+  nsIPrincipal* principal = node->NodePrincipal();
-+  if (!principal) {
-+    PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+           ("CheckEventForXSS:no principal"));
-+    return false;
-+  }
-+  nsRefPtr<nsXSSFilter> xss;
-+  principal->GetXSSFilter(getter_AddRefs(xss));
-+  if (!xss) {
-+    return false;
-+  }
-+  // 2. event body
-+  nsCOMPtr<nsIJSEventListener> jslistener =
-+    do_QueryInterface(aListenerStruct->mListener);
-+  if (!jslistener) {
-+    PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+           ("CheckEventForXSS:no nsIJSEventListener"));
-+    return false;
-+  }
-+  nsCOMPtr<nsIContent> content =
-+    do_QueryInterface(jslistener->GetEventTarget());
-+  if (!content) {
-+    PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+           ("CheckEventForXSS:no content"));
-+    return false;
-+  }
-+  nsAutoString handlerBody;
-+  PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+         ("CheckEventForXSS:content && doc"));
-+  nsresult rv = content->GetAttr(kNameSpaceID_None, aListenerStruct->mTypeAtom, handlerBody);
-+  if (NS_FAILED(rv)) {
-+    NS_ERROR("Failed to retrieve attribute value for xss check");
-+    return false;
-+  }
-+
-+  // 3. actual check
-+  aListenerStruct->mXSSChecked = true;
-+  if (!xss->PermitsEventListener(handlerBody)) {
-+    PR_LOG(gXssPRLog, PR_LOG_DEBUG,
-+           ("CheckEventForXSS:XSSFilter blocked event handler."));
-+    aListenerStruct->mXSSResult = true;
-+    return true;
-+  }
-+  aListenerStruct->mXSSResult = false;
-+  return false;
-+}
-+
-+/**
  * Causes a check for event listeners and processing by them if they exist.
  * @param an event listener
  */
  
- void
- nsEventListenerManager::HandleEventInternal(nsPresContext* aPresContext,
-                                             nsEvent* aEvent,
-                                             nsIDOMEvent** aDOMEvent,
-diff --git a/content/events/src/nsEventListenerManager.h b/content/events/src/nsEventListenerManager.h
---- a/content/events/src/nsEventListenerManager.h
-+++ b/content/events/src/nsEventListenerManager.h
-@@ -34,16 +34,19 @@ class nsIDocument;
- struct nsListenerStruct
- {
-   nsRefPtr<nsIDOMEventListener> mListener;
-   PRUint32                      mEventType;
-   nsCOMPtr<nsIAtom>             mTypeAtom;
-   PRUint16                      mFlags;
-   bool                          mHandlerIsString;
-   bool                          mWrappedJS;
-+  bool                          mXSSHandlerWasString;
-+  bool                          mXSSChecked;
-+  bool                          mXSSResult;
- 
-   nsIJSEventListener* GetJSListener() const {
-     return (mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) ?
-       static_cast<nsIJSEventListener *>(mListener.get()) : nsnull;
-   }
- 
-   ~nsListenerStruct()
-   {
-@@ -296,16 +299,20 @@ protected:
-   nsISupports*                              mTarget;  //WEAK
-   nsCOMPtr<nsIAtom>                         mNoListenerForEventAtom;
- 
-   static PRUint32                           mInstanceCount;
-   static jsid                               sAddListenerID;
- 
-   friend class nsEventTargetChainItem;
-   static PRUint32                           sCreatedCount;
-+
-+private:
-+  static bool IsEventXSS(nsListenerStruct* aListenerStruct,
-+                         nsPIDOMEventTarget* aCurrentTarget);
- };
- 
- /**
-  * NS_AddSystemEventListener() is a helper function for implementing
-  * nsIDOMEventTarget::AddSystemEventListener().
-  */
- inline nsresult
- NS_AddSystemEventListener(nsIDOMEventTarget* aTarget,
 diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
 --- a/docshell/base/nsDocShell.cpp
 +++ b/docshell/base/nsDocShell.cpp
 @@ -3954,16 +3954,21 @@ nsDocShell::DisplayLoadError(nsresult aE
          formatStrCount = 1;
          error.AssignLiteral("netTimeout");
      }
      else if (NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION == aError) {