Skip to content

Commit

Permalink
Improve find binary outside sandbox
Browse files Browse the repository at this point in the history
  • Loading branch information
SemaiCZE committed Apr 6, 2018
1 parent 49950bc commit 39b965c
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 28 deletions.
25 changes: 20 additions & 5 deletions src/helpers/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,26 +58,41 @@ fs::path helpers::normalize_path(const fs::path &path)
return result;
}

fs::path helpers::find_path_outside_sandbox(const std::string &inside,
fs::path helpers::find_path_outside_sandbox(const std::string &inside_path,
const std::string &sandbox_chdir,
std::vector<std::tuple<std::string, std::string, sandbox_limits::dir_perm>> &bound_dirs)
std::vector<std::tuple<std::string, std::string, sandbox_limits::dir_perm>> &bound_dirs,
const std::string &source_dir)
{
fs::path file_path = fs::path(inside);
auto file_path = fs::path(inside_path);
if (!file_path.has_root_directory()) {
// relative path to chdir, chdir should be absolute
file_path = fs::path(sandbox_chdir) / file_path;
} else {
// for absolute paths remove /box prefix if any
std::string box_path = "/box";
if (inside_path.find(box_path) == 0) {
file_path = fs::path(inside_path.substr(box_path.length()));
}
}
file_path = normalize_path(file_path);
auto file_path_string = file_path.string();

// try to find the file in main source directory
auto source_path = fs::path(source_dir) / file_path;
if (fs::exists(source_path)) {
return source_path;
}

// absolute path in sandbox provided
// try to find the file in sandbox bound directories (absolute path in sandbox provided)
for (auto &dir : bound_dirs) {
std::string sandbox_dir_string = normalize_path(fs::path(std::get<1>(dir))).string();
std::string file_path_string = file_path.string();

if (file_path_string.find(sandbox_dir_string) == 0) {
std::string file_path_end = file_path_string.substr(sandbox_dir_string.length());
return fs::path(std::get<0>(dir)) / fs::path(file_path_end);
}
}

// not found
return fs::path();
}
6 changes: 4 additions & 2 deletions src/helpers/filesystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ namespace helpers
* @param inside path inside sandbox which will be resolved
* @param sandbox_chdir directory where sandbox is chdir-ed
* @param bound_dirs directories bound to sandbox
* @param source_dir source directory on local filesystem
* @return path outside sandbox
*/
fs::path find_path_outside_sandbox(const std::string &inside,
fs::path find_path_outside_sandbox(const std::string &inside_path,
const std::string &sandbox_chdir,
std::vector<std::tuple<std::string, std::string, sandbox_limits::dir_perm>> &bound_dirs);
std::vector<std::tuple<std::string, std::string, sandbox_limits::dir_perm>> &bound_dirs,
const std::string &source_dir);


/**
Expand Down
17 changes: 10 additions & 7 deletions src/tasks/external_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ void external_task::results_output_init()

fs::path external_task::find_path_outside_sandbox(std::string file)
{
return helpers::find_path_outside_sandbox(file, sandbox_config_->chdir, limits_->bound_dirs);
return helpers::find_path_outside_sandbox(
file, sandbox_config_->chdir, limits_->bound_dirs, (fs::path(source_dir_) / task_meta_->test_id).string());
}

void external_task::get_results_output(std::shared_ptr<task_results> result)
Expand Down Expand Up @@ -181,7 +182,7 @@ void external_task::make_binary_executable(std::string binary)
try {
binary_path = find_path_outside_sandbox(binary);
if (binary_path.empty()) {
logger_->info("Path {} not found outside sandbox, executable bits will not be set", binary);
logger_->info("Sandbox path {} not found in local filesystem, executable bit not set", binary);
return;
}

Expand All @@ -194,10 +195,12 @@ void external_task::make_binary_executable(std::string binary)
fs::permissions(
binary_path, fs::perms::add_perms | fs::perms::owner_exe | fs::perms::group_exe | fs::others_exe);
} catch (fs::filesystem_error &e) {
auto message = std::string("Failed to set executable bits for path inside '" + binary + "' and outside '" +
binary_path.string() + "'. Error: ") +
e.what();
logger_->warn(message);
throw sandbox_exception(message);
log_and_throw(logger_,
"Failed to set executable bits for path inside '",
binary,
"' and outside '",
binary_path.string(),
"'. Error: ",
e.what());
}
}
24 changes: 12 additions & 12 deletions tests/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,58 +36,58 @@ TEST(filesystem_test, normalize)
bound_dirs_type get_default_dirs()
{
bound_dirs_type dirs;
dirs.push_back(bound_dirs_tuple("/path/outside/sandbox", "/box", sandbox_limits::dir_perm::RW));
dirs.push_back(bound_dirs_tuple("/path/outside/sandbox", "/inside", sandbox_limits::dir_perm::RW));
dirs.push_back(bound_dirs_tuple("/another/path/outside/sandbox", "/execute/dir", sandbox_limits::dir_perm::RW));
return dirs;
}

TEST(filesystem_test, test_relative_1)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("inside", "chdir", dirs);
fs::path result = helpers::find_path_outside_sandbox("inside", "chdir", dirs, "");
ASSERT_EQ(fs::path().string(), result.string());
}

TEST(filesystem_test, test_relative_2)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("inside", "/box", dirs);
fs::path result = helpers::find_path_outside_sandbox("inside", "/inside", dirs, "");
ASSERT_EQ((fs::path("/path/outside/sandbox") / fs::path("inside")).string(), result.string());
}

TEST(filesystem_test, test_relative_3)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("inside/file", "/box", dirs);
fs::path result = helpers::find_path_outside_sandbox("inside/file", "/inside", dirs, "");
ASSERT_EQ((fs::path("/path/outside/sandbox") / fs::path("inside") / fs::path("file")).string(), result.string());
}

TEST(filesystem_test, test_absolute_1)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("/output.stderr", "/box/test1", dirs);
fs::path result = helpers::find_path_outside_sandbox("/output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ(fs::path().string(), result.string());
}

TEST(filesystem_test, test_absolute_2)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("/box/output.stderr", "/box/test1", dirs);
fs::path result = helpers::find_path_outside_sandbox("/inside/output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ((fs::path("/path/outside/sandbox") / fs::path("output.stderr")).string(), result.string());
}

TEST(filesystem_test, test_absolute_3)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("/box/test1/output.stderr", "/box/test1", dirs);
fs::path result = helpers::find_path_outside_sandbox("/inside/test1/output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ(
(fs::path("/path/outside/sandbox") / fs::path("test1") / fs::path("output.stderr")).string(), result.string());
}

TEST(filesystem_test, test_absolute_4)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("/box/test1/sub/output.stderr", "/box/test1", dirs);
fs::path result = helpers::find_path_outside_sandbox("/inside/test1/sub/output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ(
(fs::path("/path/outside/sandbox") / fs::path("test1") / fs::path("sub") / fs::path("output.stderr")).string(),
result.string());
Expand All @@ -96,30 +96,30 @@ TEST(filesystem_test, test_absolute_4)
TEST(filesystem_test, test_absolute_5)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("/execute/dir/sub/output.stderr", "/box/test1", dirs);
fs::path result = helpers::find_path_outside_sandbox("/execute/dir/sub/output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ((fs::path("/another/path/outside/sandbox") / fs::path("sub") / fs::path("output.stderr")).string(),
result.string());
}

TEST(filesystem_test, test_relative_6)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("../inside", "/box", dirs);
fs::path result = helpers::find_path_outside_sandbox("../indir", "/inside", dirs, "");
ASSERT_EQ(fs::path().string(), result.string());
}

TEST(filesystem_test, test_absolute_7)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result = helpers::find_path_outside_sandbox("../output.stderr", "/box/test1", dirs);
fs::path result = helpers::find_path_outside_sandbox("../output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ((fs::path("/path/outside/sandbox") / fs::path("output.stderr")).string(), result.string());
}

TEST(filesystem_test, test_absolute_8)
{
bound_dirs_type dirs = get_default_dirs();
fs::path result =
helpers::find_path_outside_sandbox("/box/test1////sub/./output.stderr", "/box/test1", dirs);
helpers::find_path_outside_sandbox("/inside/test1////sub/./output.stderr", "/inside/test1", dirs, "");
ASSERT_EQ(
(fs::path("/path/outside/sandbox") / fs::path("test1") / fs::path("sub") / fs::path("output.stderr")).string(),
result.string());
Expand Down
3 changes: 1 addition & 2 deletions tests/worker_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ TEST(worker_config, load_yaml_basic)
expected_limits.stack_size = 50000;
expected_limits.disk_size = 50;
expected_limits.disk_files = 7;
expected_limits.bound_dirs = {
std::tuple<std::string, std::string, sp>{"/usr/local/bin", "localbin", sp::RW},
expected_limits.bound_dirs = {std::tuple<std::string, std::string, sp>{"/usr/local/bin", "localbin", sp::RW},
std::tuple<std::string, std::string, sp>{"/usr/share", "share", sp::MAYBE}};
if (config.get_limits().environ_vars.at(0).first == "ISOLATE_TMP") {
expected_limits.environ_vars = {{"ISOLATE_TMP", "/tmp"}, {"ISOLATE_BOX", "/box"}};
Expand Down

0 comments on commit 39b965c

Please sign in to comment.