diff options
Diffstat (limited to 'src')
6 files changed, 78 insertions, 14 deletions
diff --git a/src/buildtool/execution_api/bazel_msg/TARGETS b/src/buildtool/execution_api/bazel_msg/TARGETS index d2a3147a..eb78742d 100644 --- a/src/buildtool/execution_api/bazel_msg/TARGETS +++ b/src/buildtool/execution_api/bazel_msg/TARGETS @@ -34,6 +34,7 @@ ] , "private-deps": [ ["src/utils/cpp", "hex_string"] + , ["src/utils/cpp", "path"] , ["src/buildtool/file_system", "file_system_manager"] , ["src/buildtool/file_system", "git_repo"] , ["src/buildtool/common", "artifact_digest_factory"] diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp index a48e7541..1992961d 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp @@ -29,6 +29,7 @@ #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/file_system/git_repo.hpp" #include "src/utils/cpp/hex_string.hpp" +#include "src/utils/cpp/path.hpp" namespace { /// \brief Serialize protobuf message to string. @@ -427,7 +428,7 @@ auto BazelMsgFactory::CreateGitTreeDigestFromLocalTree( try { if (IsSymlinkObject(type)) { auto content = FileSystemManager::ReadSymlink(full_name); - if (content) { + if (content and PathIsNonUpwards(*content)) { if (auto digest = store_symlink(*content)) { if (auto raw_id = FromHexString(digest->hash())) { entries[std::move(*raw_id)].emplace_back( diff --git a/src/buildtool/execution_api/common/tree_reader_utils.cpp b/src/buildtool/execution_api/common/tree_reader_utils.cpp index 855c1842..13775201 100644 --- a/src/buildtool/execution_api/common/tree_reader_utils.cpp +++ b/src/buildtool/execution_api/common/tree_reader_utils.cpp @@ -106,6 +106,12 @@ auto TreeReaderUtils::ReadObjectInfos(bazel_re::Directory const& dir, } for (auto const& l : dir.symlinks()) { + // check validity of symlinks + if (not PathIsNonUpwards(l.target())) { + Logger::Log( + LogLevel::Error, "found invalid symlink at {}", l.name()); + return false; + } if (not store_info(l.name(), CreateObjectInfo(hash_function, l))) { return false; } diff --git a/src/buildtool/execution_api/local/TARGETS b/src/buildtool/execution_api/local/TARGETS index 9bf9de97..a685d283 100644 --- a/src/buildtool/execution_api/local/TARGETS +++ b/src/buildtool/execution_api/local/TARGETS @@ -44,6 +44,7 @@ , ["src/buildtool/execution_api/execution_service", "cas_utils"] , ["src/buildtool/file_system", "git_repo"] , ["src/utils/cpp", "expected"] + , ["src/utils/cpp", "path"] , ["src/utils/cpp", "tmp_dir"] , ["src/buildtool/crypto", "hash_function"] ] @@ -54,7 +55,6 @@ , ["src/buildtool/file_system", "file_system_manager"] , ["src/buildtool/execution_api/utils", "outputscheck"] , ["src/buildtool/crypto", "hash_function"] - , ["src/utils/cpp", "path"] ] } , "context": diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp index 1f17ed08..10359e92 100644 --- a/src/buildtool/execution_api/local/local_action.cpp +++ b/src/buildtool/execution_api/local/local_action.cpp @@ -22,6 +22,7 @@ #include <utility> #include "src/buildtool/common/artifact_digest_factory.hpp" +#include "src/buildtool/common/protocol_traits.hpp" #include "src/buildtool/execution_api/common/tree_reader.hpp" #include "src/buildtool/execution_api/local/local_cas_reader.hpp" #include "src/buildtool/execution_api/local/local_response.hpp" @@ -30,6 +31,7 @@ #include "src/buildtool/file_system/object_type.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/system/system_command.hpp" +#include "src/utils/cpp/path.hpp" namespace { @@ -367,12 +369,21 @@ auto LocalAction::CollectOutputFileOrSymlink( return std::nullopt; } if (IsSymlinkObject(*type)) { - auto content = FileSystemManager::ReadSymlink(file_path); - if (content and local_context_.storage->CAS().StoreBlob(*content)) { - auto out_symlink = bazel_re::OutputSymlink{}; - out_symlink.set_path(local_path); - out_symlink.set_target(*content); - return out_symlink; + if (auto content = FileSystemManager::ReadSymlink(file_path)) { + // in native mode: check validity of symlink + if (ProtocolTraits::IsNative( + local_context_.storage->GetHashFunction().GetType()) and + not PathIsNonUpwards(*content)) { + Logger::Log( + LogLevel::Error, "found invalid symlink at {}", local_path); + return std::nullopt; + } + if (local_context_.storage->CAS().StoreBlob(*content)) { + auto out_symlink = bazel_re::OutputSymlink{}; + out_symlink.set_path(local_path); + out_symlink.set_target(*content); + return out_symlink; + } } } else if (IsFileObject(*type)) { @@ -406,12 +417,21 @@ auto LocalAction::CollectOutputDirOrSymlink( return std::nullopt; } if (IsSymlinkObject(*type)) { - auto content = FileSystemManager::ReadSymlink(dir_path); - if (content and local_context_.storage->CAS().StoreBlob(*content)) { - auto out_symlink = bazel_re::OutputSymlink{}; - out_symlink.set_path(local_path); - out_symlink.set_target(*content); - return out_symlink; + if (auto content = FileSystemManager::ReadSymlink(dir_path)) { + // in native mode: check validity of symlink + if (ProtocolTraits::IsNative( + local_context_.storage->GetHashFunction().GetType()) and + not PathIsNonUpwards(*content)) { + Logger::Log( + LogLevel::Error, "found invalid symlink at {}", local_path); + return std::nullopt; + } + if (local_context_.storage->CAS().StoreBlob(*content)) { + auto out_symlink = bazel_re::OutputSymlink{}; + out_symlink.set_path(local_path); + out_symlink.set_target(*content); + return out_symlink; + } } } else if (IsTreeObject(*type)) { @@ -423,6 +443,9 @@ auto LocalAction::CollectOutputDirOrSymlink( ArtifactDigestFactory::ToBazel(*digest); return out_dir; } + Logger::Log(LogLevel::Error, + "found invalid entries in directory at {}", + local_path); } else { Logger::Log( diff --git a/src/buildtool/execution_api/local/local_response.hpp b/src/buildtool/execution_api/local/local_response.hpp index 141ffbe3..1bb2480b 100644 --- a/src/buildtool/execution_api/local/local_response.hpp +++ b/src/buildtool/execution_api/local/local_response.hpp @@ -23,14 +23,18 @@ #include "fmt/core.h" #include "gsl/gsl" #include "src/buildtool/common/artifact_digest_factory.hpp" +#include "src/buildtool/common/protocol_traits.hpp" #include "src/buildtool/crypto/hash_function.hpp" #include "src/buildtool/execution_api/common/execution_response.hpp" +#include "src/buildtool/execution_api/common/tree_reader.hpp" #include "src/buildtool/execution_api/local/local_action.hpp" +#include "src/buildtool/execution_api/local/local_cas_reader.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" #include "src/buildtool/storage/storage.hpp" #include "src/utils/cpp/expected.hpp" +#include "src/utils/cpp/path.hpp" /// \brief Response of a LocalAction. class LocalResponse final : public IExecutionResponse { @@ -155,6 +159,14 @@ class LocalResponse final : public IExecutionResponse { // collect all symlinks and store them for (auto const& link : action_result.output_file_symlinks()) { try { + // in compatible mode: check symlink validity + if (not ProtocolTraits::IsNative( + storage_.GetHashFunction().GetType()) and + not PathIsNonUpwards(link.target())) { + return fmt::format( + "LocalResponse: found invalid symlink at {}", + link.path()); + } artifacts.emplace( link.path(), Artifact::ObjectInfo{ @@ -172,6 +184,14 @@ class LocalResponse final : public IExecutionResponse { } for (auto const& link : action_result.output_directory_symlinks()) { try { + // in compatible mode: check symlink validity + if (not ProtocolTraits::IsNative( + storage_.GetHashFunction().GetType()) and + not PathIsNonUpwards(link.target())) { + return fmt::format( + "LocalResponse: found invalid symlink at {}", + link.path()); + } artifacts.emplace( link.path(), Artifact::ObjectInfo{ @@ -199,6 +219,19 @@ class LocalResponse final : public IExecutionResponse { dir.path()); } try { + // in compatible mode: check validity of symlinks in dir + if (not ProtocolTraits::IsNative( + storage_.GetHashFunction().GetType())) { + auto reader = TreeReader<LocalCasReader>{&storage_.CAS()}; + auto result = reader.RecursivelyReadTreeLeafs( + *digest, "", /*include_trees=*/true); + if (not result) { + return fmt::format( + "LocalResponse: found invalid entries in directory " + "{}", + dir.path()); + } + } artifacts.emplace( dir.path(), Artifact::ObjectInfo{.digest = *std::move(digest), |