From c81ed83755183ea2bef6ec4011fc3b357b143df5 Mon Sep 17 00:00:00 2001 From: prasanna8585 Date: Sat, 18 Jul 2026 21:26:20 +0530 Subject: [PATCH] sandbox2: reject writable root in EnableSharedMountNamespace() VerifySharedMountNamespace() rejects tmpfs and /proc mounts to prevent unintended shared state when multiple Sandbox2 instances share a mount namespace, but did not check MountTree::Node::RootNode.writable. A policy built with SetRootWritable() passed validation unconditionally, and two independently-launched Sandbox2 instances sharing a mount namespace via EnableSharedMountNamespace() with a writable root end up sharing the same writable filesystem root -- confirmed end-to-end: one instance's write is directly readable by a second, separate instance. Adds the RootNode.writable() check alongside the existing tmpfs_node() check, and a regression test. --- sandboxed_api/sandbox2/BUILD | 2 ++ sandboxed_api/sandbox2/mounts.cc | 13 ++++++-- sandboxed_api/sandbox2/sandbox2_test.cc | 20 ++++++++++++ sandboxed_api/sandbox2/testcases/BUILD | 16 ++++++++++ .../sandbox2/testcases/shared_root_read.cc | 31 +++++++++++++++++++ .../sandbox2/testcases/shared_root_write.cc | 28 +++++++++++++++++ 6 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 sandboxed_api/sandbox2/testcases/shared_root_read.cc create mode 100644 sandboxed_api/sandbox2/testcases/shared_root_write.cc diff --git a/sandboxed_api/sandbox2/BUILD b/sandboxed_api/sandbox2/BUILD index 6ead61cb..c3f0e7d1 100644 --- a/sandboxed_api/sandbox2/BUILD +++ b/sandboxed_api/sandbox2/BUILD @@ -1136,6 +1136,8 @@ cc_test( "//sandboxed_api/sandbox2/testcases:minimal", "//sandboxed_api/sandbox2/testcases:pthreads", "//sandboxed_api/sandbox2/testcases:shared_memory", + "//sandboxed_api/sandbox2/testcases:shared_root_read", + "//sandboxed_api/sandbox2/testcases:shared_root_write", "//sandboxed_api/sandbox2/testcases:sleep", "//sandboxed_api/sandbox2/testcases:starve", "//sandboxed_api/sandbox2/testcases:terminate_process_group", diff --git a/sandboxed_api/sandbox2/mounts.cc b/sandboxed_api/sandbox2/mounts.cc index 8b365de2..b282e49e 100644 --- a/sandboxed_api/sandbox2/mounts.cc +++ b/sandboxed_api/sandbox2/mounts.cc @@ -94,9 +94,16 @@ absl::Status VerifyProcMount(const MountTree& mount_tree) { } absl::Status VerifySharedMountNamespace(const MountTree& mount_tree) { - if (mount_tree.has_node() && mount_tree.node().has_tmpfs_node()) { - return absl::FailedPreconditionError( - "Shared mount namespace cannot be used with tmpfs mounts."); + if (mount_tree.has_node()) { + const MountTree::Node& node = mount_tree.node(); + if (node.has_tmpfs_node()) { + return absl::FailedPreconditionError( + "Shared mount namespace cannot be used with tmpfs mounts."); + } + if (node.has_root_node() && node.root_node().writable()) { + return absl::FailedPreconditionError( + "Shared mount namespace cannot be used with a writable root."); + } } ABSL_RETURN_IF_ERROR(VerifyProcMount(mount_tree)); for (const auto& [name, subtree] : mount_tree.entries()) { diff --git a/sandboxed_api/sandbox2/sandbox2_test.cc b/sandboxed_api/sandbox2/sandbox2_test.cc index 363f35ba..0372d8f0 100644 --- a/sandboxed_api/sandbox2/sandbox2_test.cc +++ b/sandboxed_api/sandbox2/sandbox2_test.cc @@ -389,6 +389,26 @@ TEST(Sandbox2Test, SharedMountNamespaceWorks) { EXPECT_EQ(result.reason_code(), 0); } +TEST(Sandbox2Test, SharedMountNamespaceRejectsWritableRoot) { + SKIP_SANITIZERS; + + // A writable root, like tmpfs, is implicit per-instance state that + // should not be silently sharable -- EnableSharedMountNamespace() must + // reject it the same way it already rejects tmpfs. + const std::string path = + GetTestSourcePath("sandbox2/testcases/shared_root_write"); + auto executor = std::make_unique(path, std::vector{path}); + SAPI_ASSERT_OK_AND_ASSIGN( + auto policy, sandbox2::PolicyBuilder() + .DefaultAction(sandbox2::AllowAllSyscalls()) + .SetRootWritable() + .UseForkServerSharedNetNs() + .TryBuild()); + Sandbox2 sandbox(std::move(executor), std::move(policy)); + EXPECT_THAT(sandbox.EnableSharedMountNamespace(), + StatusIs(absl::StatusCode::kFailedPrecondition)); +} + TEST(SharedMemoryTest, SharedMemoryDataTransferWorks) { SKIP_SANITIZERS; const std::string path = diff --git a/sandboxed_api/sandbox2/testcases/BUILD b/sandboxed_api/sandbox2/testcases/BUILD index 54ac3be1..c5811c5d 100644 --- a/sandboxed_api/sandbox2/testcases/BUILD +++ b/sandboxed_api/sandbox2/testcases/BUILD @@ -100,6 +100,22 @@ cc_binary( features = ["fully_static_link"], ) +cc_binary( + name = "shared_root_write", + testonly = True, + srcs = ["shared_root_write.cc"], + copts = sapi_platform_copts(), + features = ["fully_static_link"], +) + +cc_binary( + name = "shared_root_read", + testonly = True, + srcs = ["shared_root_read.cc"], + copts = sapi_platform_copts(), + features = ["fully_static_link"], +) + cc_binary( name = "personality", testonly = True, diff --git a/sandboxed_api/sandbox2/testcases/shared_root_read.cc b/sandboxed_api/sandbox2/testcases/shared_root_read.cc new file mode 100644 index 00000000..f9a116ae --- /dev/null +++ b/sandboxed_api/sandbox2/testcases/shared_root_read.cc @@ -0,0 +1,31 @@ +// Copyright 2026 The Sandboxed API Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Testcase for SharedMountNamespaceLeaksWritableRoot: checks whether the +// marker file written by shared_root_write is visible from a separate +// Sandbox2 instance. + +#include +#include + +int main() { + FILE* f = fopen("/shared_root_marker", "r"); + if (f == nullptr) { + return 1; // Not found -- root is properly isolated. + } + char buf[128] = {0}; + fgets(buf, sizeof(buf), f); + fclose(f); + return strcmp(buf, "written-by-instance-A\n") == 0 ? 0 : 3; +} diff --git a/sandboxed_api/sandbox2/testcases/shared_root_write.cc b/sandboxed_api/sandbox2/testcases/shared_root_write.cc new file mode 100644 index 00000000..cbc44754 --- /dev/null +++ b/sandboxed_api/sandbox2/testcases/shared_root_write.cc @@ -0,0 +1,28 @@ +// Copyright 2026 The Sandboxed API Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Testcase for SharedMountNamespaceLeaksWritableRoot: writes a marker file +// to the sandboxee's own writable root filesystem. + +#include + +int main() { + FILE* f = fopen("/shared_root_marker", "w"); + if (f == nullptr) { + return 2; + } + fputs("written-by-instance-A\n", f); + fclose(f); + return 0; +}