Fixing renderer's access to a file from HTTP POST (after a xsite transfer).

content/common/resource_request_body.cc

Index: content/common/resource_request_body.cc
diff --git a/content/common/resource_request_body.cc b/content/common/resource_request_body.cc
index b2b0c5fd96dc41254a5ba15aca33fd8db3163174..f1fb8cebe9b165593eb18f831bc636fd5cebbaeb 100644
--- a/content/common/resource_request_body.cc
+++ b/content/common/resource_request_body.cc
@@ -48,6 +48,15 @@ void ResourceRequestBody::AppendFileSystemFileRange(
                                            expected_modification_time);
 }
 
+std::vector<base::FilePath> ResourceRequestBody::GetReferencedFiles() const {
+  std::vector<base::FilePath> result;
+  for (const auto& element : *elements()) {
+    if (element.type() == Element::TYPE_FILE)

[Łukasz Anforowicz, 2016/06/14 01:07:17]

AFAICT this is the only type that can refer to files.

There is also TYPE_FILE_FILESYSTEM that corresponds to
blink::WebHTTPBody::Element::Type::TypeFileSystemURL (with DCHECKs in some
places saying that the URI scheme should be filesystem:) and seems to be handled
by FileSystemProtocolHandler.  This type doesn't seem to deal with real paths
and therefore I think we don't need to grant any file rights based on
TYPE_FILE_FILESYSTEM elements.  OTOH, I admit that I don't fully understand what
storage/browser/fileapi does - maybe we also need to care about file access for
these elements?

[Charlie Reis, 2016/06/16 20:22:12]

Yeah, filesystem: URLs are associated with origin-specific storage, so any
renderer is allowed to request them (for now).  We'll lock them down later for
Site Isolation.

[Łukasz Anforowicz, 2016/06/16 22:05:04]

Thanks for looking into this.

[Łukasz Anforowicz, 2016/06/16 23:33:55]

Actually, I think we can land the current CL as-is (i.e. forwarding only
TYPE_FILE elements of HTTP body), but I really need to cook-up a test that also
exercises TYPE_FILE_FILESYSTEM elements (in a separate CL).  I understand that
web storage API is supposed to be restricted to a single origin, but I bet that
XSSAuditor will try to read these elements (similarly to how it reads TYPE_FILE
elements which results in a kill + similarily to how TYPE_FILE elements will [I
assume / guess] end up in PageState of the new renderer).  The argument for
safety of the transfer (of filesystem-storage elements) would be kind of similar
to the argument we made for regular files - that POST data was already handed
out to the server at the origin of the new renderer (so no additional harm would
come from also giving this data to the renderer).

[Łukasz Anforowicz, 2016/06/17 00:10:32]

Hmmm... I think that the only way to refer to filesystem-files in HTTP body is
via FormData API (basing this on looking at callers of  FormDataElement(const
KURL& fileSystemURL,...)).  AFAICT FormData API can only be used for XHRs. 
Since XHRs will not cause a navigation (and therefore will not cause a
cross-site renderer transfer) then maybe there really is no follow-up needed for
TYPE_FILE_FILESYSTEM.

[Charlie Reis, 2016/06/17 07:13:35]

Sorry for the confusion-- I was agreeing with your "therefore I think we don't
need to grant any file rights based on TYPE_FILE_FILESYSTEM elements"
conclusion.  We only need to care about TYPE_FILE in this CL.

[Łukasz Anforowicz, 2016/06/17 16:44:47]

Ok.

+      result.push_back(element.path());
+  }
+  return result;
+}
+
 ResourceRequestBody::~ResourceRequestBody() {
 }