[BlobStorage] Added back security policy for files in blobs

content/browser/blob_storage/blob_dispatcher_host.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/blob_storage/blob_dispatcher_host.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #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"
 #include "url/gurl.h"
 
 using storage::BlobStorageContext;
 using storage::BlobStorageRegistry;
 using storage::BlobTransportResult;
+using storage::DataElement;
 using storage::IPCBlobCreationCancelCode;
+using storage::FileSystemURL;
 
 namespace content {
 namespace {
 
 // These are used for UMA stats, don't change.
 enum RefcountOperation {
   BDH_DECREMENT = 0,
   BDH_INCREMENT,
   BDH_TRACING_ENUM_LAST
 };
 
 } // 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() {
   ClearHostFromBlobStorageContext();
 }
 
 void BlobDispatcherHost::OnChannelClosing() {
   ClearHostFromBlobStorageContext();
   public_blob_urls_.clear();
   blobs_inuse_map_.clear();
@@ skipping 77 matching lines @@
           context);
       Send(new BlobStorageMsg_CancelBuildingBlob(
           uuid, IPCBlobCreationCancelCode::BLOB_DEREFERENCED_WHILE_BUILDING));
     }
     return;
   }
   if (!async_builder_.IsBeingBuilt(uuid)) {
     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,
       base::Bind(&BlobDispatcherHost::SendMemoryRequest, base::Unretained(this),
                  uuid));
   SendIPCResponse(uuid, result);
 }
 
 void BlobDispatcherHost::OnMemoryItemResponse(
     const std::string& uuid,
@@ skipping 212 matching lines @@
     context->RevokePublicBlobURL(url);
   }
   for (const auto& uuid_refnum_pair : blobs_inuse_map_) {
     for (int i = 0; i < uuid_refnum_pair.second; ++i)
       context->DecrementBlobRefCount(uuid_refnum_pair.first);
   }
   async_builder_.CancelAll(context);
 }
 
 }  // namespace content