Change ChildProcessSecurityPolicy to store a list of allowed flags for

each file.

BUG=none
TEST=none


Review URL: http://codereview.chromium.org/3431032

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@60742 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 143 insertions, 37 deletions

diff --git a/chrome/browser/child_process_security_policy.cc b/chrome/browser/child_process_security_policy.cc
index 54ee31e..5068e57 100644
--- a/chrome/browser/child_process_security_policy.cc
+++ b/chrome/browser/child_process_security_policy.cc
@@ -6,6 +6,7 @@
 
 #include "base/file_path.h"
 #include "base/logging.h"
+#include "base/platform_file.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "chrome/common/bindings_policy.h"
@@ -13,6 +14,12 @@
 #include "googleurl/src/gurl.h"
 #include "net/url_request/url_request.h"
 
+const int kReadFilePermissions =
+    base::PLATFORM_FILE_OPEN |
+    base::PLATFORM_FILE_READ |
+    base::PLATFORM_FILE_EXCLUSIVE_READ |
+    base::PLATFORM_FILE_ASYNC;
+
 // The SecurityState class is used to maintain per-renderer security state
 // information.
 class ChildProcessSecurityPolicy::SecurityState {
@@ -34,9 +41,9 @@ class ChildProcessSecurityPolicy::SecurityState {
     scheme_policy_[scheme] = false;
   }
 
-  // Grant permission to upload the specified file to the web.
-  void GrantUploadFile(const FilePath& file) {
-    uploadable_files_.insert(file);
+  // Grant certain permissions to a file.
+  void GrantPermissionsForFile(const FilePath& file, int permissions) {
+    file_permissions_[file.StripTrailingSeparators()] |= permissions;
   }
 
   void GrantBindings(int bindings) {
@@ -62,10 +69,18 @@ class ChildProcessSecurityPolicy::SecurityState {
     return judgment->second;
   }
 
-  // Determine whether permission has been granted to upload file.
-  // Files that have not been granted default to being denied.
-  bool CanUploadFile(const FilePath& file) {
-    return uploadable_files_.find(file) != uploadable_files_.end();
+  // Determine if the certain permissions have been granted to a file.
+  bool HasPermissionsForFile(const FilePath& file, int permissions) {
+    FilePath current_path = file.StripTrailingSeparators();
+    FilePath last_path;
+    while (current_path != last_path) {
+      if (file_permissions_.find(current_path) != file_permissions_.end())
+        return (file_permissions_[current_path] & permissions) == permissions;
+      last_path = current_path;
+      current_path = current_path.DirName();
+    }
+
+    return false;
   }
 
   bool has_dom_ui_bindings() const {
@@ -82,7 +97,7 @@ class ChildProcessSecurityPolicy::SecurityState {
 
  private:
   typedef std::map<std::string, bool> SchemeMap;
-  typedef std::set<FilePath> FileSet;
+  typedef std::map<FilePath, int> FileMap; // bit-set of PlatformFileFlags
 
   // Maps URL schemes to whether permission has been granted or revoked:
   //   |true| means the scheme has been granted.
@@ -92,7 +107,7 @@ class ChildProcessSecurityPolicy::SecurityState {
   SchemeMap scheme_policy_;
 
   // The set of files the renderer is permited to upload to the web.
-  FileSet uploadable_files_;
+  FileMap file_permissions_;
 
   int enabled_bindings_;
 
@@ -215,15 +230,20 @@ void ChildProcessSecurityPolicy::GrantRequestURL(
   }
 }
 
-void ChildProcessSecurityPolicy::GrantUploadFile(int renderer_id,
-                                             const FilePath& file) {
+void ChildProcessSecurityPolicy::GrantReadFile(int renderer_id,
+                                               const FilePath& file) {
+  GrantPermissionsForFile(renderer_id, file, kReadFilePermissions);
+}
+
+void ChildProcessSecurityPolicy::GrantPermissionsForFile(
+    int renderer_id, const FilePath& file, int permissions) {
   AutoLock lock(lock_);
 
   SecurityStateMap::iterator state = security_state_.find(renderer_id);
   if (state == security_state_.end())
     return;
 
-  state->second->GrantUploadFile(file);
+  state->second->GrantPermissionsForFile(file, permissions);
 }
 
 void ChildProcessSecurityPolicy::GrantScheme(int renderer_id,
@@ -336,15 +356,20 @@ bool ChildProcessSecurityPolicy::CanRequestURL(
   }
 }
 
-bool ChildProcessSecurityPolicy::CanUploadFile(int renderer_id,
-                                               const FilePath& file) {
+bool ChildProcessSecurityPolicy::CanReadFile(int renderer_id,
+                                             const FilePath& file) {
+  return HasPermissionsForFile(renderer_id, file, kReadFilePermissions);
+}
+
+bool ChildProcessSecurityPolicy::HasPermissionsForFile(
+    int renderer_id, const FilePath& file, int permissions) {
   AutoLock lock(lock_);
 
   SecurityStateMap::iterator state = security_state_.find(renderer_id);
   if (state == security_state_.end())
     return false;
 
-  return state->second->CanUploadFile(file);
+  return state->second->HasPermissionsForFile(file, permissions);
 }
 
 bool ChildProcessSecurityPolicy::HasDOMUIBindings(int renderer_id) {
diff --git a/chrome/browser/child_process_security_policy.h b/chrome/browser/child_process_security_policy.h
index 6440fee..6ae4f30 100644
--- a/chrome/browser/child_process_security_policy.h
+++ b/chrome/browser/child_process_security_policy.h
@@ -69,7 +69,13 @@ class ChildProcessSecurityPolicy {
   // Whenever the user picks a file from a <input type="file"> element, the
   // browser should call this function to grant the renderer the capability to
   // upload the file to the web.
-  void GrantUploadFile(int renderer_id, const FilePath& file);
+  void GrantReadFile(int renderer_id, const FilePath& file);
+
+  // Grants certain permissions to a file. |permissions| must be a bit-set of
+  // base::PlatformFileFlags.
+  void GrantPermissionsForFile(int renderer_id,
+                               const FilePath& file,
+                               int permissions);
 
   // Grants the renderer process the capability to access URLs of the provided
   // scheme.
@@ -100,7 +106,13 @@ class ChildProcessSecurityPolicy {
   // Before servicing a renderer's request to upload a file to the web, the
   // browser should call this method to determine whether the renderer has the
   // capability to upload the requested file.
-  bool CanUploadFile(int renderer_id, const FilePath& file);
+  bool CanReadFile(int renderer_id, const FilePath& file);
+
+  // Determins if certain permissions were granted for a file. |permissions|
+  // must be a bit-set of base::PlatformFileFlags.
+  bool HasPermissionsForFile(int renderer_id,
+                             const FilePath& file,
+                             int permissions);
 
   // Returns true if the specified renderer_id has been granted DOMUIBindings.
   // The browser should check this property before assuming the renderer is
diff --git a/chrome/browser/child_process_security_policy_unittest.cc b/chrome/browser/child_process_security_policy_unittest.cc
index 46279c7..949265d 100644
--- a/chrome/browser/child_process_security_policy_unittest.cc
+++ b/chrome/browser/child_process_security_policy_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
+#include "base/platform_file.h"
 #include "chrome/browser/child_process_security_policy.h"
 #include "chrome/common/url_constants.h"
 #include "net/url_request/url_request.h"
@@ -185,30 +186,98 @@ TEST_F(ChildProcessSecurityPolicyTest, ViewSource) {
   p->Remove(kRendererID);
 }
 
-TEST_F(ChildProcessSecurityPolicyTest, CanUploadFiles) {
+TEST_F(ChildProcessSecurityPolicyTest, CanReadFiles) {
   ChildProcessSecurityPolicy* p = ChildProcessSecurityPolicy::GetInstance();
 
   p->Add(kRendererID);
 
-  EXPECT_FALSE(p->CanUploadFile(kRendererID,
+  EXPECT_FALSE(p->CanReadFile(kRendererID,
       FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  p->GrantUploadFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/passwd")));
-  EXPECT_TRUE(p->CanUploadFile(kRendererID,
+  p->GrantReadFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/passwd")));
+  EXPECT_TRUE(p->CanReadFile(kRendererID,
       FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  EXPECT_FALSE(p->CanUploadFile(kRendererID,
+  EXPECT_FALSE(p->CanReadFile(kRendererID,
       FilePath(FILE_PATH_LITERAL("/etc/shadow"))));
 
   p->Remove(kRendererID);
   p->Add(kRendererID);
 
-  EXPECT_FALSE(p->CanUploadFile(kRendererID,
+  EXPECT_FALSE(p->CanReadFile(kRendererID,
       FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  EXPECT_FALSE(p->CanUploadFile(kRendererID,
+  EXPECT_FALSE(p->CanReadFile(kRendererID,
       FilePath(FILE_PATH_LITERAL("/etc/shadow"))));
 
   p->Remove(kRendererID);
 }
 
+TEST_F(ChildProcessSecurityPolicyTest, FilePermissions) {
+  ChildProcessSecurityPolicy* p = ChildProcessSecurityPolicy::GetInstance();
+
+  // Grant permissions for a file.
+  p->Add(kRendererID);
+  FilePath file = FilePath(FILE_PATH_LITERAL("/etc/passwd"));
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_OPEN));
+
+  p->GrantPermissionsForFile(kRendererID, file,
+                             base::PLATFORM_FILE_OPEN |
+                             base::PLATFORM_FILE_READ |
+                             base::PLATFORM_FILE_WRITE |
+                             base::PLATFORM_FILE_TRUNCATE);
+  EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file,
+                                       base::PLATFORM_FILE_OPEN |
+                                       base::PLATFORM_FILE_READ |
+                                       base::PLATFORM_FILE_WRITE |
+                                       base::PLATFORM_FILE_TRUNCATE));
+  EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file,
+                                       base::PLATFORM_FILE_OPEN |
+                                       base::PLATFORM_FILE_READ));
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_CREATE));
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_CREATE |
+                                        base::PLATFORM_FILE_READ |
+                                        base::PLATFORM_FILE_WRITE |
+                                        base::PLATFORM_FILE_TRUNCATE));
+  p->Remove(kRendererID);
+
+  // Grant permissions for the directory the file is in.
+  p->Add(kRendererID);
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_OPEN));
+  p->GrantPermissionsForFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc")),
+                             base::PLATFORM_FILE_OPEN |
+                             base::PLATFORM_FILE_READ);
+  EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_OPEN));
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_READ |
+                                        base::PLATFORM_FILE_WRITE));
+  p->Remove(kRendererID);
+
+  // Grant permissions for the directory the file is in (with trailing '/').
+  p->Add(kRendererID);
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_OPEN));
+  p->GrantPermissionsForFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/")),
+                             base::PLATFORM_FILE_OPEN |
+                             base::PLATFORM_FILE_READ);
+  EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_OPEN));
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_READ |
+                                        base::PLATFORM_FILE_WRITE));
+
+  // Grant permissions for the file (should overwrite the permissions granted
+  // for the directory).
+  p->GrantPermissionsForFile(kRendererID, file, base::PLATFORM_FILE_TEMPORARY);
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file,
+                                        base::PLATFORM_FILE_OPEN));
+  EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file,
+                                       base::PLATFORM_FILE_TEMPORARY));
+  p->Remove(kRendererID);
+}
+
 TEST_F(ChildProcessSecurityPolicyTest, CanServiceInspectElement) {
   ChildProcessSecurityPolicy* p = ChildProcessSecurityPolicy::GetInstance();
 
@@ -248,11 +317,11 @@ TEST_F(ChildProcessSecurityPolicyTest, RemoveRace) {
   p->Add(kRendererID);
 
   p->GrantRequestURL(kRendererID, url);
-  p->GrantUploadFile(kRendererID, file);
+  p->GrantReadFile(kRendererID, file);
   p->GrantDOMUIBindings(kRendererID);
 
   EXPECT_TRUE(p->CanRequestURL(kRendererID, url));
-  EXPECT_TRUE(p->CanUploadFile(kRendererID, file));
+  EXPECT_TRUE(p->CanReadFile(kRendererID, file));
   EXPECT_TRUE(p->HasDOMUIBindings(kRendererID));
 
   p->Remove(kRendererID);
@@ -263,6 +332,6 @@ TEST_F(ChildProcessSecurityPolicyTest, RemoveRace) {
 
   // In this case, we default to secure behavior.
   EXPECT_FALSE(p->CanRequestURL(kRendererID, url));
-  EXPECT_FALSE(p->CanUploadFile(kRendererID, file));
+  EXPECT_FALSE(p->CanReadFile(kRendererID, file));
   EXPECT_FALSE(p->HasDOMUIBindings(kRendererID));
 }
diff --git a/chrome/browser/plugin_process_host.cc b/chrome/browser/plugin_process_host.cc
index 2032a84..093b975 100644
--- a/chrome/browser/plugin_process_host.cc
+++ b/chrome/browser/plugin_process_host.cc
@@ -379,7 +379,7 @@ void PluginProcessHost::OnAccessFiles(int renderer_id,
 
   for (size_t i = 0; i < files.size(); ++i) {
     const FilePath path = FilePath::FromWStringHack(UTF8ToWide(files[i]));
-    if (!policy->CanUploadFile(renderer_id, path)) {
+    if (!policy->CanReadFile(renderer_id, path)) {
       LOG(INFO) << "Denied unauthorized request for file " << files[i];
       *allowed = false;
       return;
diff --git a/chrome/browser/renderer_host/blob_dispatcher_host.cc b/chrome/browser/renderer_host/blob_dispatcher_host.cc
index 28a3435..8fdafe5 100644
--- a/chrome/browser/renderer_host/blob_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/blob_dispatcher_host.cc
@@ -58,7 +58,7 @@ bool BlobDispatcherHost::CheckPermission(
            blob_data->items().begin();
        iter != blob_data->items().end(); ++iter) {
     if (iter->type() == webkit_blob::BlobData::TYPE_FILE) {
-      if (!policy->CanUploadFile(process_id_, iter->file_path()))
+      if (!policy->CanReadFile(process_id_, iter->file_path()))
         return false;
     }
   }
diff --git a/chrome/browser/renderer_host/redirect_to_file_resource_handler.cc b/chrome/browser/renderer_host/redirect_to_file_resource_handler.cc
index b1470b1..a2509fb 100644
--- a/chrome/browser/renderer_host/redirect_to_file_resource_handler.cc
+++ b/chrome/browser/renderer_host/redirect_to_file_resource_handler.cc
@@ -163,7 +163,7 @@ void RedirectToFileResourceHandler::DidCreateTemporaryFile(
   file_stream_.reset(new net::FileStream(file_handle.ReleaseValue(),
                                          base::PLATFORM_FILE_WRITE |
                                          base::PLATFORM_FILE_ASYNC));
-  ChildProcessSecurityPolicy::GetInstance()->GrantUploadFile(
+  ChildProcessSecurityPolicy::GetInstance()->GrantReadFile(
       process_id_, file_path);
   host_->StartDeferredRequest(process_id_, request_id_);
 }
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index 1d1117b..9ca53bd 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -463,7 +463,7 @@ void RenderViewHost::DragTargetDragEnter(
     FilePath path = FilePath::FromWStringHack(UTF16ToWideHack(*iter));
     policy->GrantRequestURL(process()->id(),
                             net::FilePathToFileURL(path));
-    policy->GrantUploadFile(process()->id(), path);
+    policy->GrantReadFile(process()->id(), path);
   }
   Send(new ViewMsg_DragTargetDragEnter(routing_id(), drop_data, client_pt,
                                        screen_pt, operations_allowed));
@@ -675,7 +675,7 @@ void RenderViewHost::FilesSelectedInChooser(
   // Grant the security access requested to the given files.
   for (std::vector<FilePath>::const_iterator file = files.begin();
        file != files.end(); ++file) {
-    ChildProcessSecurityPolicy::GetInstance()->GrantUploadFile(
+    ChildProcessSecurityPolicy::GetInstance()->GrantReadFile(
         process()->id(), *file);
   }
   Send(new ViewMsg_RunFileChooserResponse(routing_id(), files));
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc
index 899f077..8085d93 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc
@@ -153,7 +153,7 @@ bool ShouldServiceRequest(ChildProcessInfo::ProcessType process_type,
     std::vector<net::UploadData::Element>::const_iterator iter;
     for (iter = uploads->begin(); iter != uploads->end(); ++iter) {
       if (iter->type() == net::UploadData::TYPE_FILE &&
-          !policy->CanUploadFile(child_id, iter->file_path())) {
+          !policy->CanReadFile(child_id, iter->file_path())) {
         NOTREACHED() << "Denied unauthorized upload of "
                      << iter->file_path().value();
         return false;
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 8641c6c..21264f2 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -1449,7 +1449,7 @@ void ResourceMessageFilter::OnGetFileSize(const FilePath& path,
                                           IPC::Message* reply_msg) {
   // Get file size only when the child process has been granted permission to
   // upload the file.
-  if (!ChildProcessSecurityPolicy::GetInstance()->CanUploadFile(id(), path)) {
+  if (!ChildProcessSecurityPolicy::GetInstance()->CanReadFile(id(), path)) {
     ViewHostMsg_GetFileSize::WriteReplyParams(
         reply_msg, static_cast<int64>(-1));
     Send(reply_msg);
@@ -1469,7 +1469,7 @@ void ResourceMessageFilter::OnGetFileModificationTime(const FilePath& path,
                                                       IPC::Message* reply_msg) {
   // Get file modification time only when the child process has been granted
   // permission to upload the file.
-  if (!ChildProcessSecurityPolicy::GetInstance()->CanUploadFile(id(), path)) {
+  if (!ChildProcessSecurityPolicy::GetInstance()->CanReadFile(id(), path)) {
     ViewHostMsg_GetFileModificationTime::WriteReplyParams(reply_msg,
                                                           base::Time());
     Send(reply_msg);
@@ -1508,7 +1508,7 @@ void ResourceMessageFilter::OnOpenFile(const FilePath& path,
   // Open the file only when the child process has been granted permission to
   // upload the file.
   // TODO(jianli): Do we need separate permission to control opening the file?
-  if (!ChildProcessSecurityPolicy::GetInstance()->CanUploadFile(id(), path)) {
+  if (!ChildProcessSecurityPolicy::GetInstance()->CanReadFile(id(), path)) {
     ViewHostMsg_OpenFile::WriteReplyParams(
         reply_msg,
 #if defined(OS_WIN)
@@ -1702,7 +1702,7 @@ void ResourceMessageFilter::OnAsyncOpenFile(const IPC::Message& msg,
                                             int message_id) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
-  if (!ChildProcessSecurityPolicy::GetInstance()->CanUploadFile(id(), path)) {
+  if (!ChildProcessSecurityPolicy::GetInstance()->CanReadFile(id(), path)) {
     IPC::Message* reply = new ViewMsg_AsyncOpenFile_ACK(
         msg.routing_id(), base::PLATFORM_FILE_ERROR_ACCESS_DENIED,
         IPC::InvalidPlatformFileForTransit(), message_id);