Add support for SANDBOX_NULL_TERMINATED annotation.

This handles simple cases of `const char*` parameters that are null-terminated C strings and don't have a separate size parameter. We do not handle outputs to start with.

PiperOrigin-RevId: 896024996
Change-Id: I5713924c89eadb2c6b67f625ee3db9d0875efd42

Author: Sandboxed API Team

sandboxed_api/annotations.h

@@ -72,6 +72,15 @@
 #define SANDBOX_ELEM_SIZED_BY(elem_size_arg) \
   [[clang::annotate("sandbox", "elem_sized_by", #elem_size_arg)]]
 
+// Pointer argument annotations that denote the pointee data is a
+// null-terminated C string. Only `const char*` parameters are supported.
+// The annotation does not support return types for now.
+//
+// For example:
+//   int my_open(const char* path SANDBOX_IN_PTR SANDBOX_NULL_TERMINATED);
+#define SANDBOX_NULL_TERMINATED \
+  [[clang::annotate("sandbox", "null_terminated")]]
+
 // Annotations for sandboxee and host thunks.
 // These just mean that we also emit these into the sandbox / host code.
 // They can be used to hook function calls.

sandboxed_api/annotations_unimplemented.h

@@ -39,10 +39,6 @@
 // to the sandboxee.
 #define SANDBOX_CALLBACK [[clang::annotate("sandbox", "callback")]]
 
-// TODO(b/491826252): Annotation for null-terminated strings.
-#define SANDBOX_NULL_TERMINATED \
-  [[clang::annotate("sandbox", "null_terminated")]]
-
 // TODO(b/491826267): Annotation for return values that alias a parameter.
 #define SANDBOX_ALIAS_PTR [[clang::annotate("sandbox", "alias_ptr")]]
 

sandboxed_api/tests/testcases/lwbox_sandbox.cc

@@ -3,6 +3,7 @@
 // Since we can't use both SANDBOX_IGNORE_FUNCS and SANDBOX_FUNCS in the
 // same file, we need a separate annotation file for SANDBOX_FUNCS.
 SANDBOX_FUNCS(mylib_is_sandboxed, mylib_scalar_types, mylib_add, mylib_copy,
-              mylib_copy_raw, mylib_take_enum, mylib_expected_syscall1,
-              mylib_expected_syscall2, mylib_unexpected_syscall1,
-              mylib_unexpected_syscall2, mylib_func_with_todo);
+              mylib_copy_raw, mylib_strlen, mylib_take_enum,
+              mylib_expected_syscall1, mylib_expected_syscall2,
+              mylib_unexpected_syscall1, mylib_unexpected_syscall2,
+              mylib_func_with_todo);

sandboxed_api/tests/testcases/replaced_library.cc

@@ -50,6 +50,8 @@ void mylib_copy_raw(const char* src, char* dst, size_t size) {
   memcpy(dst, src, size);
 }
 
+size_t mylib_strlen(const char* str) { return strlen(str); }
+
 int mylib_add(int x, int y) { return x + y; }
 
 // Sanitizer instrumentation may break argument value tracking.

sandboxed_api/tests/testcases/replaced_library.h

@@ -40,6 +40,8 @@ void mylib_copy_raw(const char* src SANDBOX_IN_PTR SANDBOX_ELEM_SIZED_BY(size),
                     char* dst SANDBOX_OUT_PTR SANDBOX_ELEM_SIZED_BY(size),
                     size_t size);
 
+size_t mylib_strlen(const char* str SANDBOX_IN_PTR SANDBOX_NULL_TERMINATED);
+
 MyLibEnum mylib_take_enum(MyLibEnum e);
 
 void mylib_expected_syscall1();

sandboxed_api/tests/testcases/replaced_library_test.cc

@@ -47,4 +47,12 @@ TEST(Test, Copy) {
   EXPECT_EQ(std::string(dst_buf, sizeof(dst_buf)), "telle");
 }
 
+TEST(Test, NullTerminatedStrlen) {
+  EXPECT_EQ(mylib_strlen(""), 0);
+  EXPECT_EQ(mylib_strlen("hello"), 5);
+
+  char buf[12] = {'h', 'e', 'l', 'l', 'o', '\n', 't', 'h', 'e', 'r', 'e', '\0'};
+  EXPECT_EQ(mylib_strlen(buf), 11);
+}
+
 }  // namespace

sandboxed_api/tools/clang_generator/BUILD

@@ -53,6 +53,7 @@ cc_library(
         "@abseil-cpp//absl/container:flat_hash_map",
         "@abseil-cpp//absl/container:flat_hash_set",
         "@abseil-cpp//absl/container:node_hash_set",
+        "@abseil-cpp//absl/functional:overload",
         "@abseil-cpp//absl/log",
         "@abseil-cpp//absl/log:die_if_null",
         "@abseil-cpp//absl/random",

sandboxed_api/tools/clang_generator/CMakeLists.txt

@@ -110,6 +110,7 @@ target_link_libraries(sapi_generator PUBLIC
   absl::btree
   absl::flat_hash_set
   absl::node_hash_set
+  absl::overload
   absl::random_random
   absl::status
   absl::statusor

sandboxed_api/tools/clang_generator/sandboxed_library_emitter.cc

@@ -20,9 +20,11 @@
 #include <optional>
 #include <string>
 #include <utility>
+#include <variant>
 #include <vector>
 
 #include "absl/container/flat_hash_set.h"
+#include "absl/functional/overload.h"
 #include "absl/log/log.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
@@ -61,8 +63,10 @@ constexpr absl::string_view kWrapperPrefix = "sapi_wrapper_";
 // This can be used to strip the annotations from the input string.
 std::string StripAnnotations(const std::string& input) {
   static const auto* macros_no_args = new std::vector<std::string>{
-      "SANDBOX_IN_PTR",     "SANDBOX_OUT_PTR",         "SANDBOX_INOUT_PTR",
-      "SANDBOX_OPAQUE_PTR", "SANDBOX_HOST_OPAQUE_PTR", "SANDBOX_HOST_STATE_VAR",
+      "SANDBOX_IN_PTR",          "SANDBOX_OUT_PTR",
+      "SANDBOX_INOUT_PTR",       "SANDBOX_OPAQUE_PTR",
+      "SANDBOX_HOST_OPAQUE_PTR", "SANDBOX_HOST_STATE_VAR",
+      "SANDBOX_NULL_TERMINATED",
   };
   std::string output = input;
   for (const auto& macro : *macros_no_args) {
@@ -339,6 +343,29 @@ struct StringViewArg : SandboxedLibraryEmitter::Arg {
   }
 };
 
+// A null-terminated C string. These are always "input" to the library.
+struct ConstCStrArg : SandboxedLibraryEmitter::Arg {
+  ConstCStrArg(absl::string_view name, PointerDir ptr_dir)
+      : Arg(name, "const char*") {
+    if (ptr_dir != PointerDir::kIn) {
+      LOG(FATAL) << "ConstCStrArg pointer direction must be kIn";
+    }
+  }
+
+  std::string EmitHostPreCall() const override {
+    return absl::Substitute("sapi::v::ConstCStr sapi_tmp_$0($0);\n", name_);
+  }
+  std::string EmitHostArgs() const override {
+    return absl::Substitute("sapi_tmp_$0.PtrBefore()", name_);
+  }
+  std::string EmitSandboxeeParams() const override {
+    return absl::Substitute("$0 $1", type_, name_);
+  }
+  std::string EmitSandboxeeArgs() const override {
+    return absl::Substitute("$0", name_);
+  }
+};
+
 // Handles in/out/inout pointers (singular and arrays).
 struct PointerArg : SandboxedLibraryEmitter::Arg {
   PointerArg(absl::string_view name, absl::string_view type, PointerDir ptr_dir,
@@ -1090,8 +1117,14 @@ SandboxedLibraryEmitter::ConvertImpl(absl::string_view name,
     // Check whether this pointer even needs syncing or is an opaque handle.
     if (annotations.ptr_dir == PointerDir::kSandboxOpaque ||
         annotations.ptr_dir == PointerDir::kHostOpaque) {
+      // Shouldn't be elem_sized_by or null_terminated.
+      if (!std::holds_alternative<std::monostate>(annotations.size_type)) {
+        return absl::InvalidArgumentError(absl::Substitute(
+            "pointer argument $0 is opaque and should not be sized (kind $1)",
+            name, annotations.size_type.index()));
+      }
       return std::make_unique<PointerArg>(name, type_name, *annotations.ptr_dir,
-                                          annotations.elem_sized_by);
+                                          std::nullopt);
     }
 
     if (!type->getPointeeType()->isArithmeticType()) {
@@ -1109,8 +1142,36 @@ SandboxedLibraryEmitter::ConvertImpl(absl::string_view name,
       return absl::InvalidArgumentError(
           absl::Substitute("pointer argument $0 has unknown direction", name));
     }
-    return std::make_unique<PointerArg>(name, type_name, *ptr_dir,
-                                        annotations.elem_sized_by);
+    return std::visit(
+        absl::Overload{[&](const std::monostate&)
+                           -> absl::StatusOr<SandboxedLibraryEmitter::ArgPtr> {
+                         return std::make_unique<PointerArg>(
+                             name, type_name, *ptr_dir, std::nullopt);
+                       },
+                       [&](const ElemSizedBy& elem_sized_by)
+                           -> absl::StatusOr<SandboxedLibraryEmitter::ArgPtr> {
+                         return std::make_unique<PointerArg>(
+                             name, type_name, *ptr_dir, elem_sized_by.expr);
+                       },
+                       [&](const NullTerminated&)
+                           -> absl::StatusOr<SandboxedLibraryEmitter::ArgPtr> {
+                         if (ptr_dir != PointerDir::kIn) {
+                           return absl::InvalidArgumentError(absl::Substitute(
+                               "pointer argument $0 is null-terminated but not "
+                               "an input pointer ($1)",
+                               name, *ptr_dir));
+                         }
+                         // For now only handle `const char*` (vs `char*`)
+                         if (!type->getPointeeType()->isCharType() ||
+                             !type->getPointeeType().isConstQualified()) {
+                           return absl::InvalidArgumentError(absl::Substitute(
+                               "pointer argument $0 is null-terminated but not "
+                               "a const char*",
+                               name));
+                         }
+                         return std::make_unique<ConstCStrArg>(name, *ptr_dir);
+                       }},
+        annotations.size_type);
   }
   return nullptr;
 }
@@ -1173,9 +1234,19 @@ SandboxedLibraryEmitter::ParseAnnotations(absl::string_view name,
       annotations.ptr_dir = PointerDir::kHostOpaque;
     } else if (ann.name == "elem_sized_by") {
       num_args = 2;
+      if (!std::holds_alternative<std::monostate>(annotations.size_type)) {
+        return absl::InvalidArgumentError(absl::Substitute(
+            "arg $0: cannot be both null-terminated and elem_sized_by", name));
+      }
       if (!ann.args.empty()) {
-        annotations.elem_sized_by = ann.args[0];
+        annotations.size_type = ElemSizedBy{ann.args[0]};
+      }
+    } else if (ann.name == "null_terminated") {
+      if (!std::holds_alternative<std::monostate>(annotations.size_type)) {
+        return absl::InvalidArgumentError(absl::Substitute(
+            "arg $0: cannot be both null-terminated and elem_sized_by", name));
       }
+      annotations.size_type = NullTerminated{};
     } else {
       num_args = 0;
     }

sandboxed_api/tools/clang_generator/sandboxed_library_emitter.h

@@ -18,6 +18,7 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <variant>
 #include <vector>
 
 #include "absl/container/flat_hash_map.h"
@@ -89,9 +90,15 @@ class SandboxedLibraryEmitter : public EmitterBase {
     ~Func();
   };
 
+  struct ElemSizedBy {
+    std::string expr;
+  };
+
+  struct NullTerminated {};
+
   struct Annotations {
     std::optional<PointerDir> ptr_dir;
-    std::optional<std::string> elem_sized_by;
+    std::variant<std::monostate, ElemSizedBy, NullTerminated> size_type;
   };
 
   absl::Status AddFunction(clang::FunctionDecl* decl) override;