From 90d903b98ec17eb20ad40d5b2c309779db25bbba Mon Sep 17 00:00:00 2001 From: Mounir Lamouri Date: Mon, 22 Oct 2012 16:20:38 +0100 Subject: [PATCH] Bug 777467 - Update the same-origin policy for principals to include appid/isinbrowserelement. r=bholley --- caps/idl/nsIPrincipal.idl | 30 +++++++ caps/include/nsScriptSecurityManager.h | 16 ++++ caps/src/nsPrincipal.cpp | 28 +++--- caps/src/nsScriptSecurityManager.cpp | 24 ++++++ caps/tests/mochitest/Makefile.in | 1 + .../test_app_principal_equality.html | 85 +++++++++++++++++++ 6 files changed, 174 insertions(+), 10 deletions(-) create mode 100644 caps/tests/mochitest/test_app_principal_equality.html diff --git a/caps/idl/nsIPrincipal.idl b/caps/idl/nsIPrincipal.idl index 538b5be18a2e..54a0901d36e2 100644 --- a/caps/idl/nsIPrincipal.idl +++ b/caps/idl/nsIPrincipal.idl @@ -184,17 +184,47 @@ interface nsIPrincipal : nsISerializable */ readonly attribute unsigned long appId; + %{C++ + uint32_t GetAppId() + { + uint32_t appId; + mozilla::DebugOnly rv = GetAppId(&appId); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + return appId; + } + %} + /** * Returns true iif the principal is inside a browser element. */ readonly attribute boolean isInBrowserElement; + %{C++ + bool GetIsInBrowserElement() + { + bool isInBrowserElement; + mozilla::DebugOnly rv = GetIsInBrowserElement(&isInBrowserElement); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + return isInBrowserElement; + } + %} + /** * Returns true if this principal has an unknown appId. This shouldn't * generally be used. We only expose it due to not providing the correct * appId everywhere where we construct principals. */ readonly attribute boolean unknownAppId; + + %{C++ + bool GetUnknownAppId() + { + bool unkwnownAppId; + mozilla::DebugOnly rv = GetIsInBrowserElement(&unkwnownAppId); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + return unkwnownAppId; + } + %} }; /** diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h index f4f2d73d43ba..a272d83e8e2b 100644 --- a/caps/include/nsScriptSecurityManager.h +++ b/caps/include/nsScriptSecurityManager.h @@ -348,6 +348,22 @@ public: return sStrictFileOriginPolicy; } + /** + * Returns true if the two principals share the same app attributes. + * + * App attributes are appId and the inBrowserElement flag. + * Two principals have the same app attributes if those information are + * equals. + * This method helps keeping principals from different apps isolated from + * each other. Also, it helps making sure mozbrowser (web views) and their + * parent are isolated from each other. All those entities do not share the + * same data (cookies, IndexedDB, localStorage, etc.) so we shouldn't allow + * violating that principle. + */ + static bool + AppAttributesEqual(nsIPrincipal* aFirst, + nsIPrincipal* aSecond); + private: // GetScriptSecurityManager is the only call that can make one diff --git a/caps/src/nsPrincipal.cpp b/caps/src/nsPrincipal.cpp index b6df185609ba..3834cb96e2b6 100644 --- a/caps/src/nsPrincipal.cpp +++ b/caps/src/nsPrincipal.cpp @@ -263,34 +263,42 @@ nsPrincipal::GetOrigin(char **aOrigin) NS_IMETHODIMP nsPrincipal::Equals(nsIPrincipal *aOther, bool *aResult) { + *aResult = false; + if (!aOther) { NS_WARNING("Need a principal to compare this to!"); - *aResult = false; return NS_OK; } - if (this != aOther) { - - // Codebases are equal if they have the same origin. - *aResult = - NS_SUCCEEDED(nsScriptSecurityManager::CheckSameOriginPrincipal(this, - aOther)); + if (aOther == this) { + *aResult = true; return NS_OK; } - *aResult = true; + // Codebases are equal if they have the same origin. + *aResult = NS_SUCCEEDED( + nsScriptSecurityManager::CheckSameOriginPrincipal(this, aOther)); return NS_OK; } NS_IMETHODIMP nsPrincipal::EqualsIgnoringDomain(nsIPrincipal *aOther, bool *aResult) { - if (this == aOther) { + *aResult = false; + + if (!aOther) { + NS_WARNING("Need a principal to compare this to!"); + return NS_OK; + } + + if (aOther == this) { *aResult = true; return NS_OK; } - *aResult = false; + if (!nsScriptSecurityManager::AppAttributesEqual(this, aOther)) { + return NS_OK; + } nsCOMPtr otherURI; nsresult rv = aOther->GetURI(getter_AddRefs(otherURI)); diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index e265351b45a0..8bc6420ad8da 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -890,6 +890,10 @@ nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject, if (aSubject == aObject) return NS_OK; + if (!AppAttributesEqual(aSubject, aObject)) { + return NS_ERROR_DOM_PROP_ACCESS_DENIED; + } + // Default to false, and change if that turns out wrong. bool subjectSetDomain = false; bool objectSetDomain = false; @@ -950,6 +954,26 @@ nsScriptSecurityManager::HashPrincipalByOrigin(nsIPrincipal* aPrincipal) return SecurityHashURI(uri); } +/* static */ bool +nsScriptSecurityManager::AppAttributesEqual(nsIPrincipal* aFirst, + nsIPrincipal* aSecond) +{ + MOZ_ASSERT(aFirst && aSecond, "Don't pass null pointers!"); + + uint32_t firstAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID; + if (!aFirst->GetUnknownAppId()) { + firstAppId = aFirst->GetAppId(); + } + + uint32_t secondAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID; + if (!aSecond->GetUnknownAppId()) { + secondAppId = aSecond->GetAppId(); + } + + return ((firstAppId == secondAppId) && + (aFirst->GetIsInBrowserElement() == aSecond->GetIsInBrowserElement())); +} + nsresult nsScriptSecurityManager::CheckSameOriginDOMProp(nsIPrincipal* aSubject, nsIPrincipal* aObject, diff --git a/caps/tests/mochitest/Makefile.in b/caps/tests/mochitest/Makefile.in index c7d0b47a7627..fa748a8c4a4e 100644 --- a/caps/tests/mochitest/Makefile.in +++ b/caps/tests/mochitest/Makefile.in @@ -16,6 +16,7 @@ MOCHITEST_FILES = test_bug423375.html \ test_bug292789.html \ test_bug470804.html \ test_disallowInheritPrincipal.html \ + test_app_principal_equality.html \ $(NULL) # extendedOrigin test doesn't work on Windows, see bug 776296. diff --git a/caps/tests/mochitest/test_app_principal_equality.html b/caps/tests/mochitest/test_app_principal_equality.html new file mode 100644 index 000000000000..2dddc55c0999 --- /dev/null +++ b/caps/tests/mochitest/test_app_principal_equality.html @@ -0,0 +1,85 @@ + + + + + + Test app principal's equality + + + + +Mozilla Bug 777467 +

+ + +
+
+
+ +