Resource requests from Save-Page-As should go through CanRequestURL checks.

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index 11c590b98aa2bec8ae5c790b2321705d30bdfee8..6d86b06d47b3fa442ce52d6e0728716e1705740c 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -1910,6 +1910,17 @@ void ResourceDispatcherHostImpl::BeginSaveFile(const GURL& url,
       request.get(), save_item_id, save_package_id, child_id,
       render_frame_route_id, url, save_file_manager_.get()));
 
+  // Check if the renderer is permitted to request the requested URL.
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanRequestURL(child_id,
+                                                                    url)) {

[Łukasz Anforowicz, 2016/06/18 00:37:20]

This is based on a check done by ResourceDispatcherHostImpl::BeginDownload. 
AFAIK CanRequestURL(child_id) is the best we can do (i.e. 1) there are no more
granular [say, origin-based] checks and 2) no other checks apply [say,
ShouldServiceRequest checks]).

[asanka, 2016/06/20 20:24:18]

Other things to check (you've probably got this covered, but listing here to
doublecheck):

* Whether a subresource load can trigger an authentication dialog. [should be
no]
* Whether URL embedded identities are stripped from URLs for cross origin
subresource loads. [should be yes]
* Whether blob URLs work.

[Łukasz Anforowicz, 2016/06/21 16:39:34]

This sounds like something that can be tested via
/auth-basic?password=PASS&realm=REALM from embedded test server's default
handlers.  I would still need to figure out how to dismiss the dialog when
preparing the test - when opening the page to be saved.
I don't understand what this means :-(
Ok - sounds like a good test.  I hope that this can be done by simply extending
chrome/test/data/save_page/frames-runtime-changes.htm

[asanka, 2016/06/21 16:53:28]

Yeah. From the same origin, source an image or some other sub resource from
/auth-basic?...
E.g. if you have a document at foo.example.com/a.html containing <img
src="http://user:password@bar.example.com/a.png">, then the network request that
results from attempting to save a.html should strip user:password from the URL.
This is done for regular resource loads. Which is why I wanted to see if we
replicate the behavior for SavePackage downloads.

THis is also related to the internet/intranet threat mitigation discussed
elsewhere in this CL.

[Łukasz Anforowicz, 2016/07/18 23:18:26]

This was broken, but this seems to be a separate issue from the current one. 
I've opened bug https://crbug.com/629285 to track this (and I have a tentative
fix at https://codereview.chromium.org/2155333002).
I think I can say by reading the code that this stripping is only applied to
regular resource requests, but not to 1) downloads and 2) save-page-as requests.
 I am saying this, because LOAD_DO_NOT_USE_EMBEDDED_IDENTITY is only used for
regular resource requests.

This might be addressed by avoiding auth dialog altogether during save-page-as,
which is being fixed as described in my other response above.
I found out that savable_resources.cc will only report resources referred to via
http(s) or file URI - see
https://chromium.googlesource.com/chromium/src/+/222a04026cfacf032174f4aaa372...

Please open a bug if you think this is an issue.  It seems that not saving
resources from ftp URIs is an explicit engineering decision from the past.  Not
saving data: URIs should not be an issue.  Not saving blob: URIs might be an
issue, but only if the saved javascript is not working properly and not able to
recreate the blob (and the element where it is used).

+    VLOG(1) << "Denied unauthorized save of " << url.possibly_invalid_spec();
+    handler->OnResponseCompleted(
+        net::URLRequestStatus::FromError(net::ERR_ACCESS_DENIED),
+        "",        // |security_info|;  ignored by SaveFileResourceHandler.
+        nullptr);  // |defer|;  ignored by SaveFileResourceHandler.
+    return;
+  }
+
   BeginRequestInternal(std::move(request), std::move(handler));
 }