From 39b965c6d811ef25fad2a1aa49039c4d21f8096c Mon Sep 17 00:00:00 2001 From: Petr Stefan Date: Fri, 6 Apr 2018 21:40:31 +0200 Subject: [PATCH] Improve find binary outside sandbox --- src/helpers/filesystem.cpp | 25 ++++++++++++++++++++----- src/helpers/filesystem.h | 6 ++++-- src/tasks/external_task.cpp | 17 ++++++++++------- tests/filesystem.cpp | 24 ++++++++++++------------ tests/worker_config.cpp | 3 +-- 5 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/helpers/filesystem.cpp b/src/helpers/filesystem.cpp index 9c6c68d2..a607acf6 100644 --- a/src/helpers/filesystem.cpp +++ b/src/helpers/filesystem.cpp @@ -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> &bound_dirs) + std::vector> &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(); } diff --git a/src/helpers/filesystem.h b/src/helpers/filesystem.h index dcf30909..7f476a32 100644 --- a/src/helpers/filesystem.h +++ b/src/helpers/filesystem.h @@ -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> &bound_dirs); + std::vector> &bound_dirs, + const std::string &source_dir); /** diff --git a/src/tasks/external_task.cpp b/src/tasks/external_task.cpp index 9dd3e1ca..ac54f3b1 100644 --- a/src/tasks/external_task.cpp +++ b/src/tasks/external_task.cpp @@ -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 result) @@ -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; } @@ -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()); } } diff --git a/tests/filesystem.cpp b/tests/filesystem.cpp index 7a61dc14..4a1091b9 100644 --- a/tests/filesystem.cpp +++ b/tests/filesystem.cpp @@ -36,7 +36,7 @@ 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; } @@ -44,42 +44,42 @@ bound_dirs_type get_default_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()); } @@ -87,7 +87,7 @@ TEST(filesystem_test, test_absolute_3) 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()); @@ -96,7 +96,7 @@ 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()); } @@ -104,14 +104,14 @@ TEST(filesystem_test, test_absolute_5) 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()); } @@ -119,7 +119,7 @@ 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()); diff --git a/tests/worker_config.cpp b/tests/worker_config.cpp index ace90e78..874b0ed3 100644 --- a/tests/worker_config.cpp +++ b/tests/worker_config.cpp @@ -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{"/usr/local/bin", "localbin", sp::RW}, + expected_limits.bound_dirs = {std::tuple{"/usr/local/bin", "localbin", sp::RW}, std::tuple{"/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"}};