Bug 674571 - Remove xpcom/proxy use in security/manager/ssl (since this can lead to off-thread scripted QI) (r=kaie)
authorLuke Wagner <luke@mozilla.com>
Fri, 01 Jul 2011 17:33:52 -0700
changeset 74215 0cf822d12c64a1bc22782330a961c72fd34219d0
parent 74214 dd0630c44b2de23eb49b780c843de87b33ed1342
child 74216 be91fb29d950eb712abcb26929423872de68ca42
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewerskaie
bugs674571
milestone8.0a1
Bug 674571 - Remove xpcom/proxy use in security/manager/ssl (since this can lead to off-thread scripted QI) (r=kaie)
security/manager/ssl/src/nsNSSIOLayer.cpp
--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -3334,16 +3334,83 @@ done:
 
 static SECStatus
 cancel_and_failure(nsNSSSocketInfo* infoObject)
 {
   infoObject->SetCanceled(PR_TRUE);
   return SECFailure;
 }
 
+class nsIsStsHostRunnable : public nsIRunnable
+{
+ public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIRUNNABLE
+
+  nsIsStsHostRunnable(const nsCOMPtr<nsIStrictTransportSecurityService> &stss)
+    : stss(stss), stsEnabled(PR_FALSE), nsrv(NS_ERROR_UNEXPECTED)
+  {}
+
+  nsXPIDLCString hostName;
+
+  nsresult GetResult(PRBool &b) const { b = stsEnabled; return nsrv; }
+
+ private:
+  nsCOMPtr<nsIStrictTransportSecurityService> stss;
+  PRBool stsEnabled;
+  nsresult nsrv;
+};
+
+NS_IMPL_THREADSAFE_ISUPPORTS1(nsIsStsHostRunnable,
+                              nsIRunnable)
+
+NS_IMETHODIMP nsIsStsHostRunnable::Run()
+{
+  nsrv = stss->IsStsHost(hostName, &stsEnabled);
+  return NS_OK;
+}
+
+class nsNotifyCertProblemRunnable : public nsIRunnable
+{
+ public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIRUNNABLE
+
+  nsNotifyCertProblemRunnable(nsIInterfaceRequestor *cb,
+                              nsIInterfaceRequestor *csi,
+                              nsSSLStatus* status,
+                              const nsCString &hostWithPortString)
+  : cb(cb),
+    csi(csi),
+    status(status),
+    hostWithPortString(hostWithPortString),
+    suppressMessage(PR_FALSE)
+  {}
+
+  PRBool GetSuppressMessage() { return suppressMessage; }
+
+ private:
+  nsIInterfaceRequestor* cb;
+  nsIInterfaceRequestor* csi;
+  nsSSLStatus* status;
+  const nsCString& hostWithPortString;
+  PRBool suppressMessage;
+};
+
+NS_IMPL_THREADSAFE_ISUPPORTS1(nsNotifyCertProblemRunnable,
+                              nsIRunnable)
+
+NS_IMETHODIMP nsNotifyCertProblemRunnable::Run()
+{
+  nsCOMPtr<nsIBadCertListener2> bcl = do_GetInterface(cb);
+  if (bcl)
+    bcl->NotifyCertProblem(csi, status, hostWithPortString, &suppressMessage);
+  return NS_OK;
+}
+
 static SECStatus
 nsNSSBadCertHandler(void *arg, PRFileDesc *sslSocket)
 {
   // cert was revoked, don't do anything else
   // Calling cancel_and_failure is not necessary, and would be wrong,
   // [for errors other than the ones explicitly handled below,] 
   // because it suppresses error reporting.
   if (PR_GetError() == SEC_ERROR_REVOKED_CERTIFICATE)
@@ -3517,31 +3584,35 @@ nsNSSBadCertHandler(void *arg, PRFileDes
   remaining_display_errors = collected_errors;
 
   // Enforce Strict-Transport-Security for hosts that are "STS" hosts:
   // connections must be dropped when there are any certificate errors
   // (STS Spec section 7.3).
 
   nsCOMPtr<nsIStrictTransportSecurityService> stss
     = do_GetService(NS_STSSERVICE_CONTRACTID);
-  nsCOMPtr<nsIStrictTransportSecurityService> proxied_stss;
-
-  nsrv = NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
-                              NS_GET_IID(nsIStrictTransportSecurityService),
-                              stss, NS_PROXY_SYNC,
-                              getter_AddRefs(proxied_stss));
-  NS_ENSURE_SUCCESS(nsrv, SECFailure);
+
+  nsCOMPtr<nsIsStsHostRunnable> runnable(new nsIsStsHostRunnable(stss));
+  if (!runnable)
+    return SECFailure;
 
   // now grab the host name to pass to the STS Service
-  nsXPIDLCString hostName;
-  nsrv = infoObject->GetHostName(getter_Copies(hostName));
+  nsrv = infoObject->GetHostName(getter_Copies(runnable->hostName));
+  NS_ENSURE_SUCCESS(nsrv, SECFailure);
+
+  nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+  if (!mainThread)
+    return SECFailure;
+
+  // Dispatch SYNC since the result is used below
+  nsrv = mainThread->Dispatch(runnable, NS_DISPATCH_SYNC);
   NS_ENSURE_SUCCESS(nsrv, SECFailure);
 
   PRBool strictTransportSecurityEnabled;
-  nsrv = proxied_stss->IsStsHost(hostName, &strictTransportSecurityEnabled);
+  nsrv = runnable->GetResult(strictTransportSecurityEnabled);
   NS_ENSURE_SUCCESS(nsrv, SECFailure);
 
   if (!strictTransportSecurityEnabled) {
     nsCOMPtr<nsICertOverrideService> overrideService =
       do_GetService(NS_CERTOVERRIDE_CONTRACTID);
     // it is fine to continue without the nsICertOverrideService
 
     PRUint32 overrideBits = 0;
@@ -3571,43 +3642,33 @@ nsNSSBadCertHandler(void *arg, PRFileDes
     PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("Strict-Transport-Security is violated: untrusted transport layer\n"));
   }
 
   // Ok, this is a full stop.
   // First, deliver the technical details of the broken SSL status,
   // giving the caller a chance to suppress the error messages.
 
   PRBool suppressMessage = PR_FALSE;
-  nsresult rv;
 
   // Try to get a nsIBadCertListener2 implementation from the socket consumer.
   nsCOMPtr<nsIInterfaceRequestor> cb;
   infoObject->GetNotificationCallbacks(getter_AddRefs(cb));
   if (cb) {
-    nsCOMPtr<nsIInterfaceRequestor> callbacks;
-    NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
-                         NS_GET_IID(nsIInterfaceRequestor),
-                         cb,
-                         NS_PROXY_SYNC,
-                         getter_AddRefs(callbacks));
-
-    nsCOMPtr<nsIBadCertListener2> bcl = do_GetInterface(callbacks);
-    if (bcl) {
-      nsCOMPtr<nsIBadCertListener2> proxy_bcl;
-      NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
-                           NS_GET_IID(nsIBadCertListener2),
-                           bcl,
-                           NS_PROXY_SYNC,
-                           getter_AddRefs(proxy_bcl));
-      if (proxy_bcl) {
-        nsIInterfaceRequestor *csi = static_cast<nsIInterfaceRequestor*>(infoObject);
-        rv = proxy_bcl->NotifyCertProblem(csi, status, hostWithPortString, 
-                                          &suppressMessage);
-      }
-    }
+    nsIInterfaceRequestor *csi = static_cast<nsIInterfaceRequestor*>(infoObject);
+
+    nsCOMPtr<nsNotifyCertProblemRunnable> runnable(
+        new nsNotifyCertProblemRunnable(cb, csi, status, hostWithPortString));
+    if (!runnable)
+      return SECFailure;
+
+    // Dispatch SYNC since the result is used below
+    nsrv = mainThread->Dispatch(runnable, NS_DISPATCH_SYNC);
+    NS_ENSURE_SUCCESS(nsrv, SECFailure);
+
+    suppressMessage = runnable->GetSuppressMessage();
   }
 
   nsCOMPtr<nsIRecentBadCertsService> recentBadCertsService = 
     do_GetService(NS_RECENTBADCERTS_CONTRACTID);
 
   if (recentBadCertsService) {
     recentBadCertsService->AddBadCert(hostWithPortStringUTF16, status);
   }