From 97433a48ab57400051989cce07234422935fbf4d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 8 Oct 2008 09:16:27 -0400 Subject: [PATCH] Bug 454850. Make sure that whenever nsPrincipal::Equals would return true for a pair of principals their nsPrincipal::GetHashValue returns are also equal. r+sr=bzbarsky --- caps/include/nsScriptSecurityManager.h | 4 ++ caps/src/nsPrincipal.cpp | 4 +- caps/src/nsScriptSecurityManager.cpp | 29 +++++++++ netwerk/base/public/nsNetUtil.h | 81 ++++++++++++++++++-------- 4 files changed, 90 insertions(+), 28 deletions(-) diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h index b3d51fbef424..6430a7775e57 100644 --- a/caps/include/nsScriptSecurityManager.h +++ b/caps/include/nsScriptSecurityManager.h @@ -402,14 +402,18 @@ public: * false otherwise. */ static PRBool SecurityCompareURIs(nsIURI* aSourceURI, nsIURI* aTargetURI); + static PRUint32 SecurityHashURI(nsIURI* aURI); static nsresult ReportError(JSContext* cx, const nsAString& messageTag, nsIURI* aSource, nsIURI* aTarget); + static nsresult CheckSameOriginPrincipal(nsIPrincipal* aSubject, nsIPrincipal* aObject, PRBool aIsCheckConnect); + static PRUint32 + HashPrincipalByOrigin(nsIPrincipal* aPrincipal); static PRBool GetStrictFileOriginPolicy() diff --git a/caps/src/nsPrincipal.cpp b/caps/src/nsPrincipal.cpp index 330821245a66..ca98ec67eecc 100755 --- a/caps/src/nsPrincipal.cpp +++ b/caps/src/nsPrincipal.cpp @@ -719,9 +719,7 @@ nsPrincipal::GetHashValue(PRUint32* aValue) *aValue = nsCRT::HashCode(mCert->fingerprint.get()); } else { - nsCAutoString str; - mCodebase->GetSpec(str); - *aValue = nsCRT::HashCode(str.get()); + *aValue = nsScriptSecurityManager::HashPrincipalByOrigin(this); } return NS_OK; diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index a08d6d9b3b92..50440b4b1647 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -319,6 +319,14 @@ nsScriptSecurityManager::SecurityCompareURIs(nsIURI* aSourceURI, return NS_SecurityCompareURIs(aSourceURI, aTargetURI, sStrictFileOriginPolicy); } +// SecurityHashURI is consistent with SecurityCompareURIs because NS_SecurityHashURI +// is consistent with NS_SecurityCompareURIs. See nsNetUtil.h. +PRUint32 +nsScriptSecurityManager::SecurityHashURI(nsIURI* aURI) +{ + return NS_SecurityHashURI(aURI); +} + NS_IMETHODIMP nsScriptSecurityManager::GetChannelPrincipal(nsIChannel* aChannel, nsIPrincipal** aPrincipal) @@ -900,6 +908,27 @@ nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject, return NS_ERROR_DOM_PROP_ACCESS_DENIED; } +// It's important that +// +// CheckSameOriginPrincipal(A, B, PR_FALSE) == NS_OK +// +// imply +// +// HashPrincipalByOrigin(A) == HashPrincipalByOrigin(B) +// +// if principals A and B could ever be used as keys in a hashtable. +// Violation of this invariant leads to spurious failures of hashtable +// lookups. See bug 454850. + +/*static*/ PRUint32 +nsScriptSecurityManager::HashPrincipalByOrigin(nsIPrincipal* aPrincipal) +{ + nsCOMPtr uri; + aPrincipal->GetDomain(getter_AddRefs(uri)); + if (!uri) + aPrincipal->GetURI(getter_AddRefs(uri)); + return SecurityHashURI(uri); +} nsresult nsScriptSecurityManager::CheckSameOriginDOMProp(nsIPrincipal* aSubject, diff --git a/netwerk/base/public/nsNetUtil.h b/netwerk/base/public/nsNetUtil.h index ad69da89d6d2..bcb26df374b0 100644 --- a/netwerk/base/public/nsNetUtil.h +++ b/netwerk/base/public/nsNetUtil.h @@ -48,6 +48,7 @@ #include "nsCOMPtr.h" #include "prio.h" // for read/write flags, permissions, etc. +#include "nsCRT.h" #include "nsIURI.h" #include "nsIInputStream.h" #include "nsIOutputStream.h" @@ -1466,6 +1467,60 @@ NS_OfflineAppAllowed(nsIURI *aURI, nsIPrefBranch *aPrefBranch = nsnull) return allowed; } +static inline PRInt32 +GetEffectivePort(nsIURI* aURI) +{ + PRInt32 port; + + nsCOMPtr baseURI = NS_GetInnermostURI(aURI); + if (NS_SUCCEEDED(baseURI->GetPort(&port)) && port != -1) + return port; + + nsCAutoString scheme; + if (NS_FAILED(baseURI->GetScheme(scheme))) + return -1; + + return NS_GetDefaultPort(scheme.get()); +} + +// NS_SecurityHashURI must return the same hash value for any two URIs that +// compare equal according to NS_SecurityCompareURIs. Unfortunately, in the +// case of files, it's not clear we can do anything better than returning +// the schemeHash, so hashing files degenerates to storing them in a list. +inline PRUint32 +NS_SecurityHashURI(nsIURI* aURI) +{ + nsCOMPtr baseURI = NS_GetInnermostURI(aURI); + + nsCAutoString scheme; + PRUint32 schemeHash = 0; + if (NS_SUCCEEDED(baseURI->GetScheme(scheme))) + schemeHash = nsCRT::HashCode(scheme.get()); + + // TODO figure out how to hash file:// URIs + if (scheme.EqualsLiteral("file")) + return schemeHash; // sad face + + if (scheme.EqualsLiteral("imap") || + scheme.EqualsLiteral("mailbox") || + scheme.EqualsLiteral("news")) + { + nsCAutoString spec; + PRUint32 specHash = baseURI->GetSpec(spec); + if (NS_SUCCEEDED(specHash)) + specHash = nsCRT::HashCode(spec.get()); + return specHash; + } + + nsCAutoString host; + PRUint32 hostHash = 0; + if (NS_SUCCEEDED(baseURI->GetHost(host))) + hostHash = nsCRT::HashCode(host.get()); + + // XOR to combine hash values + return schemeHash ^ hostHash ^ GetEffectivePort(aURI); +} + inline PRBool NS_SecurityCompareURIs(nsIURI* aSourceURI, nsIURI* aTargetURI, @@ -1563,31 +1618,7 @@ NS_SecurityCompareURIs(nsIURI* aSourceURI, return PR_FALSE; } - // Compare ports - PRInt32 targetPort; - nsresult rv = targetBaseURI->GetPort(&targetPort); - PRInt32 sourcePort; - if (NS_SUCCEEDED(rv)) - rv = sourceBaseURI->GetPort(&sourcePort); - PRBool result = NS_SUCCEEDED(rv) && targetPort == sourcePort; - // If the port comparison failed, see if either URL has a - // port of -1. If so, replace -1 with the default port - // for that scheme. - if (NS_SUCCEEDED(rv) && !result && - (sourcePort == -1 || targetPort == -1)) - { - PRInt32 defaultPort = NS_GetDefaultPort(targetScheme.get()); - if (defaultPort == -1) - return PR_FALSE; // No default port for this scheme - - if (sourcePort == -1) - sourcePort = defaultPort; - else if (targetPort == -1) - targetPort = defaultPort; - result = targetPort == sourcePort; - } - - return result; + return GetEffectivePort(targetBaseURI) == GetEffectivePort(sourceBaseURI); } #endif // !nsNetUtil_h__