[tbb-commits] [tor-browser/tor-browser-60.1.0esr-8.0-1] Bug 26456: HTTP .onion sites inherit previous page's

[gk at torproject.org]

commit 3c5e1db5bc2131872bca5ec766522787beeee869
Author: Richard Pospesel <richard at torproject.org>
Date:   Wed Aug 15 12:51:16 2018 -0700

    Bug 26456: HTTP .onion sites inherit previous page's
     certificate information
    
    A side-effect of marking the state of HTTP onion pages as 'secure' is
    that they go through the EvaluateAndUpdateSecurityState code path in
    nsSecureBrowserUIImpl.
    
    The previous implementation would just leave the SSLStatus as-is when
    receiving an SSL 'info' object which could not be QueryInterface'd to
    an nsISSLStatusProvider.  For secure SSL pages, this code-path would
    never occur, but for secure onion pages it would.
    
    This would result in the previous page's SSLStatus hanging around when
    transitioning to an HTTP onion site with the previous HTTPS's SSL info
    remaining for the JavaScript chrome to pull in and display.
    
    This patch tweaks the EvaluateAndUpdateSecurityState to correctly clear
    the nsSecureBrowserUIImpl's owned nsISSLStatusProvider object in this
    scenario.
---
 security/manager/ssl/nsSecureBrowserUIImpl.cpp | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.cpp b/security/manager/ssl/nsSecureBrowserUIImpl.cpp
index 336901f0226e..dc39f6e1d131 100644
--- a/security/manager/ssl/nsSecureBrowserUIImpl.cpp
+++ b/security/manager/ssl/nsSecureBrowserUIImpl.cpp
@@ -376,7 +376,6 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest,
   mNewToplevelIsEV = false;
 
   bool updateStatus = false;
-  nsCOMPtr<nsISSLStatus> temp_SSLStatus;
 
   mNewToplevelSecurityState =
     GetSecurityStateFromSecurityInfoAndRequest(info, aRequest);
@@ -387,21 +386,26 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest,
 
   nsCOMPtr<nsISSLStatusProvider> sp(do_QueryInterface(info));
   if (sp) {
-    // Ignore result
+    // Ignore GetSSLStatus result
     updateStatus = true;
-    (void) sp->GetSSLStatus(getter_AddRefs(temp_SSLStatus));
-    if (temp_SSLStatus) {
+    (void) sp->GetSSLStatus(getter_AddRefs(mSSLStatus));
+    if (mSSLStatus) {
       bool aTemp;
-      if (NS_SUCCEEDED(temp_SSLStatus->GetIsExtendedValidation(&aTemp))) {
+      if (NS_SUCCEEDED(mSSLStatus->GetIsExtendedValidation(&aTemp))) {
         mNewToplevelIsEV = aTemp;
       }
     }
+  } else {
+    // No new SSL status, so clear out previous mSSLStatus and mark it for update
+    if (mSSLStatus != nullptr) {
+      mSSLStatus = nullptr;
+      updateStatus = true;
+    }
+    // also clear out EV flag
+    mNewToplevelIsEV = false;
   }
 
   mNewToplevelSecurityStateKnown = true;
-  if (updateStatus) {
-    mSSLStatus = temp_SSLStatus;
-  }
   MOZ_LOG(gSecureDocLog, LogLevel::Debug,
          ("SecureUI:%p: remember securityInfo %p\n", this,
           info));