Commit 09a6fe60 authored by Joel Hockey's avatar Joel Hockey Committed by Commit Bot

Map VM file drop paths to avoid spoofing host paths

Not all paths in a VM can be mapped to a path in the host.
Paths in crostini homedir and /mnt/chromeos are fine. E.g.

 /mnt/chromeos/MyFiles/file => <cyrptohome>/MyFiles/file.

But /etc/mime.types is not accessible to the host and cannot be mapped
to a valid host path, so we will map it as
'vmfile:termina:/etc/mime.types'.

If the drop target is the crostini container, we will do the reverse
mapping and provide file:///etc/mime.types, but if we fail to do
the reverse mapping for the drop target (e.g. drop target is arc or
Plugin VM), we will clear the path from the drop data.

Since chrome accesses OSExchangeData directly without allowing exo to do
any reverse mapping, chrome will attempt to use a path such as
file:///vmfile:termina:/etc/mime.types which is intended to fail.

Bug: 1144138
Bug: 1146319
Change-Id: I91c62b5183f054311cba2ef474dda0f12766db83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546677Reviewed-by: default avatarJason Lin <lxj@google.com>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829159}
parent a8739ae4
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/memory/ref_counted_memory.h" #include "base/memory/ref_counted_memory.h"
#include "base/strings/strcat.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
...@@ -50,6 +51,7 @@ namespace { ...@@ -50,6 +51,7 @@ namespace {
constexpr char kMimeTypeArcUriList[] = "application/x-arc-uri-list"; constexpr char kMimeTypeArcUriList[] = "application/x-arc-uri-list";
constexpr char kMimeTypeTextUriList[] = "text/uri-list"; constexpr char kMimeTypeTextUriList[] = "text/uri-list";
constexpr char kUriListSeparator[] = "\r\n"; constexpr char kUriListSeparator[] = "\r\n";
constexpr char kVmFileScheme[] = "vmfile";
bool IsArcWindow(const aura::Window* window) { bool IsArcWindow(const aura::Window* window) {
return static_cast<ash::AppType>(window->GetProperty( return static_cast<ash::AppType>(window->GetProperty(
...@@ -134,21 +136,30 @@ void ShareAndSend(aura::Window* target, ...@@ -134,21 +136,30 @@ void ShareAndSend(aura::Window* target,
vm_name = plugin_vm::kPluginVmName; vm_name = plugin_vm::kPluginVmName;
} }
const std::string vm_prefix =
base::StrCat({kVmFileScheme, ":", vm_name, ":"});
std::vector<std::string> lines_to_send; std::vector<std::string> lines_to_send;
auto* share_path = guest_os::GuestOsSharePath::GetForProfile(primary_profile); auto* share_path = guest_os::GuestOsSharePath::GetForProfile(primary_profile);
std::vector<base::FilePath> paths_to_share; std::vector<base::FilePath> paths_to_share;
for (auto& info : files) { for (auto& info : files) {
// Crostini and PluginVm converts to path inside VM. For other app types, or
// if we fail to convert, we just keep the original path.
base::FilePath path_to_send = info.path; base::FilePath path_to_send = info.path;
if ((is_crostini || is_plugin_vm) && if (is_crostini || is_plugin_vm) {
file_manager::util::ConvertFileSystemURLToPathInsideVM( // Check if it is a path inside the VM: 'vmfile:<vm_name>:'.
primary_profile, info.url, vm_mount, if (base::StartsWith(info.path.value(), vm_prefix,
/*map_crostini_home=*/is_crostini, &path_to_send) && base::CompareCase::SENSITIVE)) {
!share_path->IsPathShared(vm_name, info.path)) { path_to_send =
// Keep a record of any paths which are not yet shared. base::FilePath(info.path.value().substr(vm_prefix.size()));
paths_to_share.emplace_back(info.path); } else if (file_manager::util::ConvertFileSystemURLToPathInsideVM(
primary_profile, info.url, vm_mount,
/*map_crostini_home=*/is_crostini, &path_to_send)) {
// Convert to path inside the VM and check if the path needs sharing.
if (!share_path->IsPathShared(vm_name, info.path))
paths_to_share.emplace_back(info.path);
} else {
LOG(WARNING) << "Could not convert path " << info.path;
continue;
}
} }
lines_to_send.emplace_back(net::FilePathToFileURL(path_to_send).spec()); lines_to_send.emplace_back(net::FilePathToFileURL(path_to_send).spec());
} }
...@@ -178,11 +189,19 @@ class ChromeFileHelper : public exo::FileHelper { ...@@ -178,11 +189,19 @@ class ChromeFileHelper : public exo::FileHelper {
aura::Window* toplevel = source->GetToplevelWindow(); aura::Window* toplevel = source->GetToplevelWindow();
bool is_crostini = crostini::IsCrostiniWindow(toplevel); bool is_crostini = crostini::IsCrostiniWindow(toplevel);
bool is_plugin_vm = plugin_vm::IsPluginVmAppWindow(toplevel); bool is_plugin_vm = plugin_vm::IsPluginVmAppWindow(toplevel);
base::FilePath vm_mount;
std::string vm_name;
if (is_crostini) {
vm_mount = crostini::ContainerChromeOSBaseDirectory();
vm_name = crostini::kCrostiniDefaultVmName;
} else if (is_plugin_vm) {
vm_mount = plugin_vm::ChromeOSBaseDirectory();
vm_name = plugin_vm::kPluginVmName;
}
std::string lines(data.begin(), data.end()); std::string lines(data.begin(), data.end());
std::vector<ui::FileInfo> filenames; std::vector<ui::FileInfo> filenames;
// Until we have mapping for other VMs, only accept from arc.
if (!IsArcWindow(source))
return filenames;
base::FilePath path; base::FilePath path;
storage::FileSystemURL url; storage::FileSystemURL url;
...@@ -191,16 +210,19 @@ class ChromeFileHelper : public exo::FileHelper { ...@@ -191,16 +210,19 @@ class ChromeFileHelper : public exo::FileHelper {
if (!net::FileURLToFilePath(GURL(line), &path)) if (!net::FileURLToFilePath(GURL(line), &path))
continue; continue;
if (is_crostini && // Convert the VM path to a path in the host if possible (in homedir or
file_manager::util::ConvertPathInsideVMToFileSystemURL( // /mnt/chromeos for crostini; in //ChromeOS for Plugin VM), otherwise
primary_profile, path, crostini::ContainerChromeOSBaseDirectory(), // prefix with 'vmfile:<vm_name>:' to avoid VMs spoofing host paths.
/*map_crostini_home=*/true, &url)) { // E.g. crostini /etc/mime.types => vmfile:termina:/etc/mime.types.
path = url.path(); if (is_crostini || is_plugin_vm) {
} else if (is_plugin_vm && if (file_manager::util::ConvertPathInsideVMToFileSystemURL(
file_manager::util::ConvertPathInsideVMToFileSystemURL( primary_profile, path, vm_mount,
primary_profile, path, plugin_vm::ChromeOSBaseDirectory(), /*map_crostini_home=*/is_crostini, &url)) {
/*map_crostini_home=*/false, &url)) { path = url.path();
path = url.path(); } else {
path = base::FilePath(
base::StrCat({kVmFileScheme, ":", vm_name, ":", path.value()}));
}
} }
filenames.emplace_back(ui::FileInfo(path, base::FilePath())); filenames.emplace_back(ui::FileInfo(path, base::FilePath()));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment