Skip to content

Commit

Permalink
policybuilder: Add AnchorPathAbsolute utility function.
Browse files Browse the repository at this point in the history
This utility function can be used by users of the `PolicyBuilder` to create an absolute path anchored to a defined `base`.

* If the path passed as `relative_path` is absolute; The path will be returned as is.
* If the passed path is relative and `base` is not provided; The path will be resolved relative to the current working directory.
* If the passed path is relative and an absolute `base` is provided; The path will
be resolved relative to `base`.
* If both, `relative_path` and `base` are relative, then first `base` will be
resolved relative to the current working directory, and then `relative_path` will be resolved relative to `base`.

In all cases where `relative_path` is relative; Any path elements such as dot-dot, dot, slash-slash, and trailing slash are cleaned up as string manipulation (no syscall is performed for path normalization). The result is then anchored to the base directory.

In case of an error, an empty path will be returned. Calling `AddFile`, `AddFileAt`, `AddDirectory`, and `AddDirectoryAt` with empty strings does not result in an error on policy compilation.

PiperOrigin-RevId: 719239498
Change-Id: Ie51cfb9171ee74934d2b663414c83b6245707aed
  • Loading branch information
okunz authored and copybara-github committed Jan 24, 2025
1 parent 7f62fb9 commit 7cb9a03
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 1 deletion.
3 changes: 2 additions & 1 deletion sandboxed_api/sandbox2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1153,10 +1153,11 @@ cc_test(
":policy",
":policybuilder",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_googletest//:gtest_main",
"@com_google_sandboxed_api//sandboxed_api/sandbox2/util:bpf_helper",
"@com_google_sandboxed_api//sandboxed_api/util:file_base",
"@com_google_sandboxed_api//sandboxed_api/util:fileops",
"@com_google_sandboxed_api//sandboxed_api/util:status_matchers",
],
)
Expand Down
2 changes: 2 additions & 0 deletions sandboxed_api/sandbox2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,8 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
sandbox2::bpf_helper
sandbox2::policy
sandbox2::policybuilder
sapi::file_base
sapi::fileops
sapi::testing
sapi::status_matchers
sapi::test_main
Expand Down
42 changes: 42 additions & 0 deletions sandboxed_api/sandbox2/policybuilder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
#include "sandboxed_api/sandbox2/syscall.h"
#include "sandboxed_api/sandbox2/trace_all_syscalls.h"
#include "sandboxed_api/sandbox2/util/bpf_helper.h"
#include "sandboxed_api/util/fileops.h"
#include "sandboxed_api/util/path.h"

#if defined(SAPI_X86_64)
Expand Down Expand Up @@ -94,6 +95,7 @@ namespace sandbox2 {
namespace {

namespace file = ::sapi::file;
namespace fileops = ::sapi::file_util::fileops;

// Validates that the path is absolute and canonical.
absl::StatusOr<std::string> ValidatePath(absl::string_view path,
Expand Down Expand Up @@ -1774,4 +1776,44 @@ PolicyBuilder& PolicyBuilder::SetError(const absl::Status& status) {
return *this;
}

std::string PolicyBuilder::AnchorPathAbsolute(absl::string_view relative_path,
absl::string_view base) {
if (relative_path.empty()) {
LOG(ERROR) << "Passed relative_path is empty";
return "";
}

if (file::IsAbsolutePath(relative_path)) {
VLOG(3) << "Nothing to do, relative_path is absolute";
return std::string(relative_path);
}

std::string clean_path = file::CleanPath(relative_path);
if (absl::StartsWith(clean_path, "../") || clean_path == "..") {
LOG(ERROR)
<< "Anchored path would be outside of base because relative_path: '"
<< relative_path << "' starts with '..'";
return "";
}

if (file::IsAbsolutePath(base)) {
return file::CleanPath(file::JoinPath(base, clean_path));
}

std::string cwd = fileops::GetCWD();
if (cwd.empty()) {
LOG(ERROR) << "Failed to get current working directory";
return "";
}

if (base.empty()) {
VLOG(1) << "Using current working directory as base is empty";
// CWD is guaranteed to exist and clean_path is guaranteed to not start with
// '..'.
return file::CleanPath(file::JoinPath(cwd, clean_path));
}

return file::CleanPath(file::JoinPath(cwd, base, clean_path));
}

} // namespace sandbox2
23 changes: 23 additions & 0 deletions sandboxed_api/sandbox2/policybuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,29 @@ class PolicyBuilder final {

const Mounts& mounts() const { return mounts_; }

// Returns the absolute path for the given `relative_path`.
//
// If `relative_path` is absolute, it will be returned as is and `base` will
// be ignored.
//
// If `relative_path` is relative and `base` is not provided, it will be
// resolved relative to the current working directory.
//
// If `relative_path` is relative and an absolute `base` is provided, it will
// be resolved relative to `base`.
//
// If both, `relative_path` and `base` are relative, then first `base` will be
// resolved relative to the current working directory, and then
// `relative_path` will be resolved relative to `base`.
//
// In all cases where `relative_path` is relative, non-canonical paths will be
// canonicalized and the result must be anchored to the base directory. If the
// resulting path is outside the base directory, an error will be returned.
//
// On ERROR, such as `relative_path` is empty, an empty string is returned.
static std::string AnchorPathAbsolute(absl::string_view relative_path,
absl::string_view base = {});

private:
friend class PolicyBuilderPeer; // For testing
friend class StackTracePeer;
Expand Down
66 changes: 66 additions & 0 deletions sandboxed_api/sandbox2/policybuilder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <initializer_list>
#include <memory>
#include <string>
#include <tuple>
#include <utility>
#include <vector>

Expand All @@ -31,6 +32,8 @@
#include "sandboxed_api/sandbox2/allow_unrestricted_networking.h"
#include "sandboxed_api/sandbox2/policy.h"
#include "sandboxed_api/sandbox2/util/bpf_helper.h"
#include "sandboxed_api/util/fileops.h"
#include "sandboxed_api/util/path.h"
#include "sandboxed_api/util/status_matchers.h"

namespace sandbox2 {
Expand All @@ -47,11 +50,14 @@ class PolicyBuilderPeer {

namespace {

namespace fileops = ::sapi::file_util::fileops;

using ::sapi::IsOk;
using ::sapi::StatusIs;
using ::testing::Eq;
using ::testing::Lt;
using ::testing::StartsWith;
using ::testing::StrEq;

TEST(PolicyBuilderTest, Testpolicy_size) {
ssize_t last_size = 0;
Expand Down Expand Up @@ -154,6 +160,66 @@ TEST(PolicyBuilderTest, ApisWithPathValidation) {
IsOk());
}

TEST(PolicyBuilderTest, TestAnchorPathAbsolute) {
const std::initializer_list<
std::tuple<absl::string_view, absl::string_view, std::string>>
kTestCases = {
// relative_path is empty:
{"", "/base", ""}, // Error: relative path is empty
{"", "", ""}, // Error: relative path is empty

// relative_path is absolute:
{"/a/b/c/d", "/base", "/a/b/c/d"},
{"/a/../../../../../etc/passwd", "/base",
"/a/../../../../../etc/passwd"},
{"/a/b/c/d", "base", "/a/b/c/d"},
{"/a/b/c/d", "", "/a/b/c/d"},

// base is absolute:
{"a/b/c/d", "/base", "/base/a/b/c/d"},
{"a/b/c/d/", "/base", "/base/a/b/c/d"},
{"a/b/c//d", "/base", "/base/a/b/c/d"},
{"a/b/../d/", "/base", "/base/a/d"},
{"a/./b/c/", "/base", "/base/a/b/c"},
{"./a/b/c/", "/base", "/base/a/b/c"},
{"..foobar", "/base", "/base/..foobar"},
{"a/b/c/d", "/base/../foo/bar",
"/foo/bar/a/b/c/d"}, // Not an error because base is trusted.
{"a/../../d/", "/base", ""}, // Error: can't guarantee anchor
{"../a/b/c/", "/base", ""}, // Error: can't guarantee anchor
{"..", "/base", ""}, // Error: can't guarantee anchor

// base path is empty:
{"a/b/c", "", fileops::GetCWD() + "/a/b/c"},
{"a/../../../../c", "", ""}, // Error: can't guarantee anchor

// base is relative:
{"a/b/c/d", "base", fileops::GetCWD() + "/base/a/b/c/d"},
{"a/b/c/d/", "base", fileops::GetCWD() + "/base/a/b/c/d"},
{"a/b/c//d", "base", fileops::GetCWD() + "/base/a/b/c/d"},
{"a/b/../d/", "base", fileops::GetCWD() + "/base/a/d"},
{"a/./b/c/", "base", fileops::GetCWD() + "/base/a/b/c"},
{"./a/b/c/", "base", fileops::GetCWD() + "/base/a/b/c"},
{"..foobar", "base", fileops::GetCWD() + "/base/..foobar"},
{"a/../../d/", "base", ""}, // Error: can't guarantee anchor
{"../a/b/c/", "base", ""}, // Error: can't guarantee anchor
{"..", "base", ""}, // Error: can't guarantee anchor
{"a/b/c", ".base/foo/", fileops::GetCWD() + "/.base/foo/a/b/c"},
{"a/b/c", "./base/foo", fileops::GetCWD() + "/base/foo/a/b/c"},
{"a/b/c", "base/foo/../bar", fileops::GetCWD() + "/base/bar/a/b/c"},
{"a/b/c", "base/foo//bar/",
fileops::GetCWD() + "/base/foo/bar/a/b/c"},
{"a/b/c", "..base/foo", fileops::GetCWD() + "/..base/foo/a/b/c"},
{"a/b/c", "../base/foo",
sapi::file::CleanPath(fileops::GetCWD() + "/../base/foo/a/b/c")},
{"a/b/c", "..",
sapi::file::CleanPath(fileops::GetCWD() + "/../a/b/c")},
};
for (auto const& [path, base, result] : kTestCases) {
EXPECT_THAT(PolicyBuilder::AnchorPathAbsolute(path, base), StrEq(result));
}
}

TEST(PolicyBuilderTest, TestCanOnlyBuildOnce) {
PolicyBuilder b;
ASSERT_THAT(b.TryBuild(), IsOk());
Expand Down

0 comments on commit 7cb9a03

Please sign in to comment.