From 06cd4cbcc285901ca27dc132c1e3cebf80056c5d Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Tue, 29 Sep 2026 12:13:59 +0800 Subject: [PATCH] Reject Paths with an Embedded NUL When Confining Extraction is_path_within_root compared each component with "..", so a name such as "..\0" passed the check. The filesystem calls stop at the NUL and act on a shorter path than the one that was checked: a symlink target read from a plugin archive as raw bytes was created as "..", pointing out of the plugin directory. A path containing a NUL is now rejected before anything touches the filesystem, which covers every caller, including entry names taken from the Unicode Path extra field. --- src/libslic3r/Utils.hpp | 2 +- src/libslic3r/utils.cpp | 3 +++ tests/libslic3r/test_utils.cpp | 13 ++++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libslic3r/Utils.hpp b/src/libslic3r/Utils.hpp index 9913111dea..eff7f51c2a 100644 --- a/src/libslic3r/Utils.hpp +++ b/src/libslic3r/Utils.hpp @@ -256,7 +256,7 @@ extern bool is_gallery_file(const std::string& path, char const* type); extern bool is_shapes_dir(const std::string& dir); //BBS: add json support extern bool is_json_file(const std::string& path); -// True if rel_path is relative, has no ".." component and, joined to root, still resolves inside it. +// True if rel_path is relative, has no ".." component or embedded NUL and, joined to root, still resolves inside it. // Both '/' and '\\' are treated as separators on every platform, so an archive rejected on one OS // is rejected on all of them. extern bool is_path_within_root(const std::string &rel_path, const boost::filesystem::path &root); diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 56f0ffc5fb..303476f023 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -1093,6 +1093,9 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p auto is_separator = [](char c) { return c == '/' || c == '\\'; }; if (rel_path.empty() || is_separator(rel_path.front()) || (rel_path.size() > 1 && rel_path[1] == ':')) return false; + // The filesystem calls stop at a NUL, so they would act on a shorter path than the one checked here. + if (rel_path.find('\0') != std::string::npos) + return false; for (size_t start = 0; start <= rel_path.size();) { size_t end = start; while (end < rel_path.size() && !is_separator(rel_path[end])) diff --git a/tests/libslic3r/test_utils.cpp b/tests/libslic3r/test_utils.cpp index 6428464f89..e2b220d482 100644 --- a/tests/libslic3r/test_utils.cpp +++ b/tests/libslic3r/test_utils.cpp @@ -270,6 +270,15 @@ TEST_CASE("is_path_within_root treats Windows-specific name forms the same on ev } } +TEST_CASE("is_path_within_root rejects a name with an embedded NUL", "[utils]") { + ScopedTemporaryDir tmp; + // The filesystem calls stop at the NUL, so they would act on a different path than the one checked. + const std::string name = GENERATE(std::string("..\0", 3), std::string("..\0x/file.json", 14), std::string("sub/..\0x", 8), + std::string("file.json\0", 10), std::string("\0file.json", 10)); + CAPTURE(name.size()); + CHECK_FALSE(is_path_within_root(name, tmp.path())); +} + TEST_CASE("is_symlink_target_within_root accepts relative targets that stay inside the root", "[utils]") { ScopedTemporaryDir tmp; const auto [link, target] = GENERATE(std::make_pair(std::string("Versions/Current"), std::string("A")), @@ -293,7 +302,9 @@ TEST_CASE("is_symlink_target_within_root rejects absolute targets and targets th std::make_pair(std::string("link"), std::string("../outside")), std::make_pair(std::string("sub/link"), std::string("../../outside")), std::make_pair(std::string("sub/link"), std::string("x/../../../outside")), - std::make_pair(std::string("sub/link"), std::string("..\\..\\outside"))); + std::make_pair(std::string("sub/link"), std::string("..\\..\\outside")), + // symlink() stops at the NUL, so this target would be created as "..". + std::make_pair(std::string("link"), std::string("..\0", 3))); CAPTURE(link, target); CHECK_FALSE(is_symlink_target_within_root(link, target, tmp.path())); }