arc: Filter socket path
BUG=b:219093809
TEST=cros_workon_make --test --board=octopus-arc-r arcvm-mojo-proxy
TEST=tast run arc.Boot.vm
Change-Id: I812080dc2e35c0997cde393bf22b29349dc94826
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3463477
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
(cherry picked from commit 15bb9bf953469b8c34d3cae1d1b50b41aea57031)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3476764
Auto-Submit: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Satoshi Niwa <niwa@chromium.org>
Commit-Queue: Satoshi Niwa <niwa@chromium.org>
diff --git a/arc/vm/mojo_proxy/mojo_proxy.cc b/arc/vm/mojo_proxy/mojo_proxy.cc
index ead8565..7441d25 100644
--- a/arc/vm/mojo_proxy/mojo_proxy.cc
+++ b/arc/vm/mojo_proxy/mojo_proxy.cc
@@ -26,6 +26,9 @@
namespace arc {
namespace {
+// Path to the ARC bridge socket path.
+constexpr char kArcBridgeSocketPath[] = "/run/chrome/arc_bridge.sock";
+
std::unique_ptr<LocalFile> CreateFile(
base::ScopedFD fd,
arc_proxy::FileDescriptor::Type fd_type,
@@ -68,6 +71,7 @@
MojoProxy::MojoProxy(Delegate* delegate)
: delegate_(delegate),
+ expected_socket_paths_{base::FilePath(kArcBridgeSocketPath)},
next_handle_(delegate_->GetType() == Type::SERVER ? 1 : -1),
next_cookie_(delegate_->GetType() == Type::SERVER ? 1 : -1) {
// Note: this needs to be initialized after weak_factory_, which is
@@ -351,15 +355,18 @@
}
bool MojoProxy::OnConnectRequest(arc_proxy::ConnectRequest* request) {
+ base::FilePath path(request->path());
+ if (expected_socket_paths_.count(path) == 0) {
+ LOG(ERROR) << "Unexpected socket path: " << path;
+ return false;
+ }
arc_proxy::MojoMessage reply;
auto* response = reply.mutable_connect_response();
-
+ response->set_cookie(request->cookie());
// Currently, this actually uses only on ArcBridgeService's initial
// connection establishment, and the request comes from the guest to the host
// including the |path|.
- // TODO(hidehiko): Consider allowlist the path which is allowed to access.
- auto result = ConnectUnixDomainSocket(base::FilePath(request->path()));
- response->set_cookie(request->cookie());
+ auto result = ConnectUnixDomainSocket(path);
response->set_error_code(result.first);
if (result.first == 0) {
response->set_handle(RegisterFileDescriptor(
diff --git a/arc/vm/mojo_proxy/mojo_proxy.h b/arc/vm/mojo_proxy/mojo_proxy.h
index f5906b0..e6d8f7b 100644
--- a/arc/vm/mojo_proxy/mojo_proxy.h
+++ b/arc/vm/mojo_proxy/mojo_proxy.h
@@ -10,10 +10,12 @@
#include <deque>
#include <map>
#include <memory>
+#include <set>
#include <string>
#include <vector>
#include <base/files/file_descriptor_watcher_posix.h>
+#include <base/files/file_path.h>
#include <base/files/scoped_file.h>
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
@@ -21,10 +23,6 @@
#include "arc/vm/mojo_proxy/message.pb.h"
-namespace base {
-class FilePath;
-}
-
namespace arc {
class LocalFile;
@@ -76,6 +74,10 @@
~MojoProxy();
+ void AddExpectedSocketPathForTesting(const base::FilePath& path) {
+ expected_socket_paths_.insert(path);
+ }
+
// Registers the |fd| whose type is |fd_type| to watch.
// When |handle| is not 0, associates the specified handle with the given FD.
// When |handle| is 0, generates a new handle value for the given FD.
@@ -169,6 +171,8 @@
base::Thread blocking_task_thread_{"BlockingThread"};
+ std::set<base::FilePath> expected_socket_paths_;
+
// Map from a |handle| (see message.proto for details) to a file
// instance wrapping the file descriptor and its watcher.
// Erasing the entry from this map should close the file descriptor
diff --git a/arc/vm/mojo_proxy/mojo_proxy_test.cc b/arc/vm/mojo_proxy/mojo_proxy_test.cc
index b76c43f..998fec5 100644
--- a/arc/vm/mojo_proxy/mojo_proxy_test.cc
+++ b/arc/vm/mojo_proxy/mojo_proxy_test.cc
@@ -502,6 +502,7 @@
// Create unix domain socket for testing, which is connected by the following
// Connect() invocation from client side.
auto server_sock = CreateUnixDomainSocket(socket_path);
+ server()->AddExpectedSocketPathForTesting(socket_path);
// Try to follow the actual initial connection procedure.
base::RunLoop run_loop;
