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.
This commit is contained in:
Hanif Koh
2026-09-29 22:37:43 +08:00
parent 2ae1fdbd01
commit 06cd4cbcc2
3 changed files with 16 additions and 2 deletions
+1 -1
View File
@@ -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);
+3
View File
@@ -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]))
+12 -1
View File
@@ -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()));
}