[BlobStorage] Added back security policy for files in blobs

content/browser/blob_storage/blob_dispatcher_host.cc

Index: content/browser/blob_storage/blob_dispatcher_host.cc
diff --git a/content/browser/blob_storage/blob_dispatcher_host.cc b/content/browser/blob_storage/blob_dispatcher_host.cc
index 2c43055a5ecf3c7b9ff45cb2269bbfb5910aed77..e512aae39e71d9976bf0e3c4be34b2fa6a84ef30 100644
--- a/content/browser/blob_storage/blob_dispatcher_host.cc
+++ b/content/browser/blob_storage/blob_dispatcher_host.cc
@@ -10,10 +10,14 @@
 #include "base/metrics/histogram_macros.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
+#include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/fileapi/browser_file_system_helper.h"
 #include "content/common/fileapi/webblob_messages.h"
 #include "ipc/ipc_platform_file.h"
 #include "storage/browser/blob/blob_storage_context.h"
 #include "storage/browser/blob/blob_transport_result.h"
+#include "storage/browser/fileapi/file_system_context.h"
+#include "storage/browser/fileapi/file_system_url.h"
 #include "storage/common/blob_storage/blob_item_bytes_request.h"
 #include "storage/common/blob_storage/blob_item_bytes_response.h"
 #include "storage/common/data_element.h"
@@ -22,7 +26,9 @@
 using storage::BlobStorageContext;
 using storage::BlobStorageRegistry;
 using storage::BlobTransportResult;
+using storage::DataElement;
 using storage::IPCBlobCreationCancelCode;
+using storage::FileSystemURL;
 
 namespace content {
 namespace {
@@ -37,8 +43,13 @@ enum RefcountOperation {
 } // namespace
 
 BlobDispatcherHost::BlobDispatcherHost(
+    int process_id,
+    storage::FileSystemContext* file_system_context,
     ChromeBlobStorageContext* blob_storage_context)
     : BrowserMessageFilter(BlobMsgStart),
+      process_id_(process_id),
+      file_system_context_(file_system_context),
+      security_policy_(ChildProcessSecurityPolicyImpl::GetInstance()),

[kinuko, 2016/08/05 15:27:31]

Do we want to store this pointer in this class?  It feels just calling
ChildProcessSecurityPolicy::GetInstance() could be fine too

[dmurph, 2016/08/05 19:12:45]

Sure, I was just modeling fileapi_message_filter.

       blob_storage_context_(blob_storage_context) {}
 
 BlobDispatcherHost::~BlobDispatcherHost() {
@@ -136,6 +147,31 @@ void BlobDispatcherHost::OnStartBuildingBlob(
     SendIPCResponse(uuid, BlobTransportResult::BAD_IPC);
     return;
   }
+
+  for (const DataElement& item : descriptions) {
+    if (item.type() == storage::DataElement::TYPE_FILE_FILESYSTEM) {
+      FileSystemURL filesystem_url(
+          file_system_context_->CrackURL(item.filesystem_url()));
+      if (!FileSystemURLIsValid(file_system_context_, filesystem_url) ||
+          !security_policy_->CanReadFileSystemFile(process_id_,
+                                                   filesystem_url)) {
+        async_builder_.CancelBuildingBlob(
+            uuid, IPCBlobCreationCancelCode::FILE_WRITE_FAILED, context);
+        Send(new BlobStorageMsg_CancelBuildingBlob(
+            uuid, IPCBlobCreationCancelCode::FILE_WRITE_FAILED));

[kinuko, 2016/08/05 15:27:30]

(I think we were previously just ignoring such item, but now we're canceling
this at all?  Not sure if it causes undesirable (or more desirable)
consequences...)

[dmurph, 2016/08/05 19:12:45]

Well, strangely this is the same. We now need to cancel it so the renderer stops
sending us memory, and we still have the blob reference saved, it's just now in
the empty 'broken' state.

+        return;
+      }
+    }
+    if (item.type() == storage::DataElement::TYPE_FILE &&
+        !security_policy_->CanReadFile(process_id_, item.path())) {
+      async_builder_.CancelBuildingBlob(
+          uuid, IPCBlobCreationCancelCode::FILE_WRITE_FAILED, context);
+      Send(new BlobStorageMsg_CancelBuildingBlob(
+          uuid, IPCBlobCreationCancelCode::FILE_WRITE_FAILED));
+      return;
+    }
+  }
+
   // |this| owns async_builder_ so using base::Unretained(this) is safe.
   BlobTransportResult result = async_builder_.StartBuildingBlob(
       uuid, descriptions, context->memory_available(), context,