From 9f352931c2ffd8f4fe200490f160471114af7041 Mon Sep 17 00:00:00 2001
From: Mounir Lamouri <mounir.lamouri@gmail.com>
Date: Fri, 26 Oct 2012 17:53:46 +0200
Subject: [PATCH] Bug 777467 -  Update the same-origin policy for principals to
 include appid/isinbrowserelement. r=bholley a=blocking-basecamp

--HG--
extra : rebase_source : eedf0c8dd23353d474fbfd1aca3061b05e8995d3
---
 caps/idl/nsIPrincipal.idl                     | 32 ++++++-
 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, 175 insertions(+), 11 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 119c251179b1..8c5e7803c45f 100644
--- a/caps/idl/nsIPrincipal.idl
+++ b/caps/idl/nsIPrincipal.idl
@@ -184,11 +184,31 @@ interface nsIPrincipal : nsISerializable
      */
     readonly attribute unsigned long appId;
 
+    %{C++
+    uint32_t GetAppId()
+    {
+      uint32_t appId;
+      mozilla::DebugOnly<nsresult> 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<nsresult> 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
@@ -196,6 +216,16 @@ interface nsIPrincipal : nsISerializable
      */
     readonly attribute boolean unknownAppId;
 
+    %{C++
+    bool GetUnknownAppId()
+    {
+      bool unkwnownAppId;
+      mozilla::DebugOnly<nsresult> rv = GetUnknownAppId(&unkwnownAppId);
+      MOZ_ASSERT(NS_SUCCEEDED(rv));
+      return unkwnownAppId;
+    }
+    %}
+
     /**
      * Returns true iff this principal is a null principal (corresponding to an
      * unknown, hence assumed minimally privileged, security context).
@@ -223,4 +253,4 @@ interface nsIExpandedPrincipal : nsISupports
    * should not be changed and should only be used ephemerally.
    */
   [noscript] readonly attribute PrincipalArray whiteList;
-};
\ No newline at end of file
+};
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 1ab68fa7f794..575467183f64 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<nsIURI> otherURI;
   nsresult rv = aOther->GetURI(getter_AddRefs(otherURI));
diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp
index 70a9cbbd8837..08a9df5b38ad 100644
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -904,6 +904,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;
@@ -964,6 +968,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 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=777467
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test app principal's equality</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=777467">Mozilla Bug 777467</a>
+<p id="display"></p>
+<script>
+  // Initialization.
+  SpecialPowers.addPermission("browser", true, document);
+  SpecialPowers.addPermission("embed-apps", true, document);
+
+  var previousPrefs = {
+    mozBrowserFramesEnabled: undefined,
+    oop_by_default: undefined,
+  };
+  try {
+    previousPrefs.mozBrowserFramesEnabled = SpecialPowers.getBoolPref('dom.mozBrowserFramesEnabled');
+  } catch(e) {}
+
+  try {
+    previousPrefs.oop_by_default = SpecialPowers.getBoolPref('dom.ipc.browser_frames.oop_by_default');
+  } catch(e) {}
+
+  SpecialPowers.setBoolPref('dom.mozBrowserFramesEnabled', true);
+  SpecialPowers.setBoolPref("dom.ipc.browser_frames.oop_by_default", false);
+</script>
+<div id="content" style="display: none;">
+  <iframe src="error404"></iframe>
+  <iframe mozbrowser src="error404"></iframe>
+  <iframe mozapp="http://example.org/manifest.webapp" mozbrowser src="error404"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for app principal's equality **/
+
+SimpleTest.waitForExplicitFinish();
+
+function canAccessDocument(win) {
+  var result = true;
+  try {
+    win.document;
+  } catch(e) {
+    result = false;
+  }
+  return result;
+}
+
+addLoadEvent(function() {
+  // Test the witness frame (we can access same-origin frame).
+  is(canAccessDocument(frames[0]), true,
+     "should be able to access the first frame");
+
+  // Test different app/browserElement frames.
+  for (var i=1; i<frames.length; ++i) {
+    is(canAccessDocument(frames[i]), false,
+       "should not be able to access the other frames");
+  }
+
+  // Cleanup.
+  if (previousPrefs.mozBrowserFramesEnabled !== undefined) {
+    SpecialPowers.setBoolPref('dom.mozBrowserFramesEnabled', previousPrefs.mozBrowserFramesEnabled);
+  }
+  if (previousPrefs.oop_by_default !== undefined) {
+    SpecialPowers.setBoolPref("dom.ipc.browser_frames.oop_by_default", previousPrefs.oop_by_default);
+  }
+
+  SpecialPowers.removePermission("browser", window.document);
+  SpecialPowers.removePermission("embed-apps", window.document);
+
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>