chromeos: Check both original and absolute paths for file: scheme

chrome/browser/net/chrome_network_delegate.cc

Index: chrome/browser/net/chrome_network_delegate.cc
diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 878fe38e7c1efc676d643edc96053bc848aa9faf..75f9cf0d4fa1f0477998d8c41eece45e701f3db1 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -448,8 +448,10 @@ bool ChromeNetworkDelegate::OnCanSetCookie(const net::URLRequest& request,
   return allow;
 }
 
-bool ChromeNetworkDelegate::OnCanAccessFile(const net::URLRequest& request,
-                                            const base::FilePath& path) const {
+bool ChromeNetworkDelegate::OnCanAccessFile(
+    const net::URLRequest& request,
+    const base::FilePath& original_path,
+    const base::FilePath& absolute_path) const {
 #if defined(OS_CHROMEOS)
   // If we're running Chrome for ChromeOS on Linux, we want to allow file
   // access. This is checked here to make IsAccessAllowed() unit-testable.
@@ -459,6 +461,15 @@ bool ChromeNetworkDelegate::OnCanAccessFile(const net::URLRequest& request,
   }
 #endif
 
+#if defined(OS_CHROMEOS)
+  // Use the absolute path on Chrome OS so that symbolic links that point to
+  // paths outside of the whitelist are rejected.
+  const base::FilePath& path = absolute_path;
+#else
+  // Use the original path on Android. Android's whitelist relies on symbolic
+  // links (ex. /sdcard is whitelisted and commonly a symbolic link).
+  const base::FilePath& path = original_path;

[mmenke, 2017/04/18 17:24:45]

These should be reviewed by someone more familiar with security on both
platforms than I am.

That having been said...Can we, both by default and on ChromeOS, check both
paths, and just make Android the exception?  We should default to more limited
permissions, unless we really need more permissive ones.

[satorux1, 2017/04/19 07:22:22]

Thanks. I'll ask jorgelo@ for a review once the patch is ready for a real code
review. :)

[satorux1, 2017/05/09 07:44:09]

Changed to check both by default.

+#endif
   return IsAccessAllowed(path, profile_path_);
 }