Fix privacy leak where script could get the path to the file selected in a file input. Bug 143220, r+sr=sicking, a=schrep.
authorbzbarsky@mit.edu
Wed, 14 Nov 2007 22:16:06 -0800
changeset 8008 b79cccab204459b9f78fafeac2a74e8c7044d1ff
parent 8007 2c34afb4d53a6af40a85055c4cdd111944cd652e
child 8009 b8b80a5ba5ff3fa3a5870d7750d1353b2323ec12
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersschrep
bugs143220
milestone1.9b2pre
Fix privacy leak where script could get the path to the file selected in a file input. Bug 143220, r+sr=sicking, a=schrep.
content/base/public/nsContentUtils.h
content/base/src/nsContentUtils.cpp
content/html/content/src/nsHTMLInputElement.cpp
content/html/content/test/Makefile.in
content/html/content/test/test_bug143220.html
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@@ -158,16 +158,22 @@ public:
 
   static PRBool   IsCallerChrome();
 
   static PRBool   IsCallerTrustedForRead();
 
   static PRBool   IsCallerTrustedForWrite();
 
   /**
+   * Check whether a caller is trusted to have aCapability.  This also
+   * checks for UniversalXPConnect in addition to aCapability.
+   */
+  static PRBool   IsCallerTrustedForCapability(const char* aCapability);
+
+  /**
    * Do not ever pass null pointers to this method.  If one of your
    * nsIContents is null, you have to decide for yourself what
    * "IsDescendantOf" really means.
    *
    * @param  aPossibleDescendant node to test for being a descendant of
    *         aPossibleAncestor
    * @param  aPossibleAncestor node to test for being an ancestor of
    *         aPossibleDescendant
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -739,28 +739,30 @@ nsContentUtils::Shutdown()
       PL_DHashTableFinish(&sEventListenerManagersHash);
       sEventListenerManagersHash.ops = nsnull;
     }
   }
 
   nsAutoGCRoot::Shutdown();
 }
 
-static PRBool IsCallerTrustedForCapability(const char* aCapability)
+// static
+PRBool
+nsContentUtils::IsCallerTrustedForCapability(const char* aCapability)
 {
   // The secman really should handle UniversalXPConnect case, since that
   // should include UniversalBrowserRead... doesn't right now, though.
   PRBool hasCap;
-  nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
-  if (NS_FAILED(ssm->IsCapabilityEnabled(aCapability, &hasCap)))
+  if (NS_FAILED(sSecurityManager->IsCapabilityEnabled(aCapability, &hasCap)))
     return PR_FALSE;
   if (hasCap)
     return PR_TRUE;
     
-  if (NS_FAILED(ssm->IsCapabilityEnabled("UniversalXPConnect", &hasCap)))
+  if (NS_FAILED(sSecurityManager->IsCapabilityEnabled("UniversalXPConnect",
+                                                      &hasCap)))
     return PR_FALSE;
   return hasCap;
 }
 
 /**
  * Checks whether two nodes come from the same origin. aTrustedNode is
  * considered 'safe' in that a user can operate on it and that it isn't
  * a js-object that implements nsIDOMNode.
--- a/content/html/content/src/nsHTMLInputElement.cpp
+++ b/content/html/content/src/nsHTMLInputElement.cpp
@@ -735,21 +735,30 @@ nsHTMLInputElement::GetValue(nsAString& 
         CopyUTF8toUTF16(mValue, aValue);
       }
     }
 
     return NS_OK;
   }
 
   if (mType == NS_FORM_INPUT_FILE) {
-    if (mFileName) {
-      aValue = *mFileName;
-    }
-    else {
-      aValue.Truncate();
+    if (nsContentUtils::IsCallerTrustedForCapability("UniversalFileRead")) {
+      if (mFileName) {
+        aValue = *mFileName;
+      }
+      else {
+        aValue.Truncate();
+      }
+    } else {
+      // Just return the leaf name
+      nsCOMPtr<nsIFile> file;
+      GetFile(getter_AddRefs(file));
+      if (!file || NS_FAILED(file->GetLeafName(aValue))) {
+        aValue.Truncate();
+      }
     }
     
     return NS_OK;
   }
 
   // Treat value == defaultValue for other input elements
   if (!GetAttr(kNameSpaceID_None, nsGkAtoms::value, aValue) &&
       (mType == NS_FORM_INPUT_RADIO || mType == NS_FORM_INPUT_CHECKBOX)) {
@@ -766,25 +775,17 @@ nsHTMLInputElement::GetValue(nsAString& 
 
 NS_IMETHODIMP 
 nsHTMLInputElement::SetValue(const nsAString& aValue)
 {
   // check security.  Note that setting the value to the empty string is always
   // OK and gives pages a way to clear a file input if necessary.
   if (mType == NS_FORM_INPUT_FILE) {
     if (!aValue.IsEmpty()) {
-      nsIScriptSecurityManager *securityManager =
-        nsContentUtils::GetSecurityManager();
-
-      PRBool enabled;
-      nsresult rv =
-        securityManager->IsCapabilityEnabled("UniversalFileRead", &enabled);
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      if (!enabled) {
+      if (!nsContentUtils::IsCallerTrustedForCapability("UniversalFileRead")) {
         // setting the value of a "FILE" input widget requires the
         // UniversalFileRead privilege
         return NS_ERROR_DOM_SECURITY_ERR;
       }
     }
     SetFileName(aValue);
   }
   else {
--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@@ -54,16 +54,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug1400.html \
 		test_bug2082.html \
 		test_bug3348.html \
 		test_bug6296.html \
 		test_bug24958.html \
 		bug100533_load.html \
 		bug100533_iframe.html \
 		test_bug100533.html \
+		test_bug143220.html \
 		test_bug237071.html \
 		bug242709_iframe.html \
 		bug242709_load.html \
 		test_bug242709.html \
 		bug277724_iframe1.html \
 		bug277724_iframe2.xhtml \
 		test_bug277724.html \
 		bug277890_iframe.html \
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug143220.html
@@ -0,0 +1,71 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=143220
+-->
+<head>
+  <title>Test for Bug 143220</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/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=143220">Mozilla Bug 143220</a>
+<p id="display">
+  <input type="file" id="i1">
+  <input type="file" id="i2">
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 143220 **/
+var leafName;
+var fullPath;
+
+function initVals() {
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  var dirSvc = Components.classes["@mozilla.org/file/directory_service;1"]
+                         .getService(Components.interfaces.nsIProperties);
+  var file = dirSvc.get("XpcomLib", Components.interfaces.nsILocalFile);
+  isnot(file, null, "Must have file here");
+
+  leafName = file.leafName;
+  fullPath = file.path;
+}
+
+function initControl1() {
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalFileRead");
+  $("i1").value = fullPath;
+  is($("i1").value, fullPath, "Should have set full path 1");
+}
+
+function initControl2() {
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  $("i2").value = fullPath;
+  is($("i2").value, fullPath, "Should have set full path 2");
+}
+
+initVals();
+
+// Check that we can't just set the value
+try {
+  $("i1").value = fullPath;
+  is(0, 1, "Should have thrown exception on set!");
+} catch(e) {
+  is($("i1").value, "", "Shouldn't have value here");
+}
+
+initControl1();
+initControl2();
+
+is($("i1").value, leafName, "Leaking full value?");
+is($("i2").value, leafName, "Leaking full value?");
+
+</script>
+</pre>
+</body>
+</html>
+