commit 3c5e1db5bc2131872bca5ec766522787beeee869 Author: Richard Pospesel richard@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));