diff --git a/sandboxed_api/sandbox2/mounts.cc b/sandboxed_api/sandbox2/mounts.cc index 73972bd..e55c382 100644 --- a/sandboxed_api/sandbox2/mounts.cc +++ b/sandboxed_api/sandbox2/mounts.cc @@ -479,6 +479,7 @@ uint64_t GetMountFlagsFor(const std::string& path) { uint64_t flags = 0; using MountPair = std::pair; for (const auto& [mount_flag, vfs_flag] : { + MountPair(MS_RDONLY, ST_RDONLY), MountPair(MS_NOSUID, ST_NOSUID), MountPair(MS_NODEV, ST_NODEV), MountPair(MS_NOEXEC, ST_NOEXEC), @@ -562,8 +563,14 @@ void MountWithDefaults(const std::string& source, const std::string& target, // Flags are ignored for a bind mount, a remount is needed to set the flags. if (extra_flags & MS_BIND) { // Get actual mount flags. - flags |= GetMountFlagsFor(target); - res = mount("", target.c_str(), "", flags | MS_REMOUNT, nullptr); + uint64_t target_flags = GetMountFlagsFor(target); + if ((target_flags & MS_RDONLY) != 0 && (flags & MS_RDONLY) == 0) { + SAPI_RAW_LOG(FATAL, + "cannot remount %s as read-write as it's on read-only dev", + target.c_str()); + } + res = mount("", target.c_str(), "", flags | target_flags | MS_REMOUNT, + nullptr); SAPI_RAW_PCHECK(res != -1, "remounting %s with flags=%s failed", target, MountFlagsToString(flags)); } diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index c42fdb7..ea35fb5 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -22,6 +22,7 @@ #include // For GRND_NONBLOCK #include // For mmap arguments #include +#include #include #include @@ -72,6 +73,15 @@ bool CheckBpfBounds(const sock_filter& filter, size_t max_jmp) { return true; } +bool IsOnReadOnlyDev(const std::string& path) { + struct statvfs vfs; + if (TEMP_FAILURE_RETRY(statvfs(path.c_str(), &vfs)) == -1) { + PLOG(ERROR) << "Could not statvfs: " << path.c_str(); + return false; + } + return vfs.f_flag & ST_RDONLY; +} + } // namespace PolicyBuilder& PolicyBuilder::AllowSyscall(uint32_t num) { @@ -939,6 +949,13 @@ PolicyBuilder& PolicyBuilder::AddFileAt(absl::string_view outside, return *this; } + if (!is_ro && IsOnReadOnlyDev(*valid_outside)) { + SetError(absl::FailedPreconditionError( + absl::StrCat("Cannot add ", outside, + " as read-write as it's on a read-only device"))); + return *this; + } + if (auto status = mounts_.AddFileAt(*valid_outside, inside, is_ro); !status.ok()) { SetError( @@ -995,6 +1012,13 @@ PolicyBuilder& PolicyBuilder::AddDirectoryAt(absl::string_view outside, return *this; } + if (!is_ro && IsOnReadOnlyDev(*valid_outside)) { + SetError(absl::FailedPreconditionError( + absl::StrCat("Cannot add ", outside, + " as read-write as it's on a read-only device"))); + return *this; + } + if (absl::Status status = mounts_.AddDirectoryAt(*valid_outside, inside, is_ro); !status.ok()) {