From 01e14e0bb7268f9d0415c71b787495bba24e1a5f Mon Sep 17 00:00:00 2001
From: Wiktor Garbacz <wiktorg@google.com>
Date: Fri, 11 Aug 2023 01:33:46 -0700
Subject: [PATCH] Fix bypass for enabling `ptrace`/`bpf`

PiperOrigin-RevId: 555847265
Change-Id: I671c0650caeefaac590d3d0030ff90e18fda6bbd
---
 sandboxed_api/sandbox2/policybuilder.cc      | 11 +++++++++--
 sandboxed_api/sandbox2/policybuilder_test.cc | 12 ++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc
index 0f7801a..d57d9f7 100644
--- a/sandboxed_api/sandbox2/policybuilder.cc
+++ b/sandboxed_api/sandbox2/policybuilder.cc
@@ -1106,6 +1106,11 @@ PolicyBuilder& PolicyBuilder::AddPolicyOnSyscalls(
   constexpr size_t kMaxShortJump = 255;
   bool last = true;
   for (auto it = std::rbegin(nums); it != std::rend(nums); ++it) {
+    if (*it == __NR_bpf || *it == __NR_ptrace) {
+      SetError(absl::InvalidArgumentError(
+          "cannot add policy for bpf/ptrace syscall"));
+      return *this;
+    }
     // If syscall is not matched try with the next one.
     uint8_t jf = 0;
     // If last syscall on the list does not match skip the policy by jumping
@@ -1479,8 +1484,10 @@ PolicyBuilder& PolicyBuilder::AddNetworkProxyHandlerPolicy() {
 }
 
 PolicyBuilder& PolicyBuilder::TrapPtrace() {
-  AddPolicyOnSyscall(__NR_ptrace, {TRAP(0)});
-  user_policy_handles_ptrace_ = true;
+  if (handled_syscalls_.insert(__NR_ptrace).second) {
+    user_policy_.insert(user_policy_.end(), {SYSCALL(__NR_ptrace, TRAP(0))});
+    user_policy_handles_ptrace_ = true;
+  }
   return *this;
 }
 
diff --git a/sandboxed_api/sandbox2/policybuilder_test.cc b/sandboxed_api/sandbox2/policybuilder_test.cc
index 8fcd7e1..d20339b 100644
--- a/sandboxed_api/sandbox2/policybuilder_test.cc
+++ b/sandboxed_api/sandbox2/policybuilder_test.cc
@@ -17,6 +17,7 @@
 #include <syscall.h>
 #include <unistd.h>
 
+#include <cerrno>
 #include <memory>
 #include <string>
 #include <utility>
@@ -48,12 +49,12 @@ class PolicyBuilderPeer {
 
 namespace {
 
+using ::sapi::IsOk;
+using ::sapi::StatusIs;
 using ::testing::Eq;
 using ::testing::Lt;
 using ::testing::StartsWith;
 using ::testing::StrEq;
-using ::sapi::IsOk;
-using ::sapi::StatusIs;
 
 TEST(PolicyBuilderTest, Testpolicy_size) {
   ssize_t last_size = 0;
@@ -152,5 +153,12 @@ TEST(PolicyBuilderTest, TestIsCopyable) {
   EXPECT_THAT(builder.TryBuild(), IsOk());
   EXPECT_THAT(copy.TryBuild(), IsOk());
 }
+
+TEST(PolicyBuilderTest, CanBypassPtrace) {
+  PolicyBuilder builder;
+  builder.AddPolicyOnSyscall(__NR_ptrace, {ALLOW})
+      .BlockSyscallWithErrno(__NR_ptrace, ENOENT);
+  EXPECT_THAT(builder.TryBuild(), Not(IsOk()));
+}
 }  // namespace
 }  // namespace sandbox2