Confine config import to the preset directory (#15608)

import_presets reduced each zip entry to a basename by stripping only
'/', so on Windows an entry named with '\' separators kept its
directory components and was extracted wherever they pointed. Strip
both separators, and reject any entry whose name still escapes the
extraction folder.

The preset name from the JSON and the bundle id from
bundle_structure.json were joined onto the preset directory unchecked
as well, which let either of them write outside it on every platform.
Both are now validated before anything is written.

The check is the is_path_within_root helper the 3MF importer already
had, moved to Utils so both importers share it. It treats '/' and '\'
as separators on every platform, so a bundle that would escape on one
OS is rejected on all of them.
This commit is contained in:
HanifKoh
2026-09-09 15:35:21 +08:00
committed by GitHub
parent 4deadc9dce
commit 8a291f9d56
5 changed files with 133 additions and 40 deletions

View File

@@ -102,45 +102,6 @@ struct ZipUnicodePathExtraField
}
};
// Validate that a relative file path does not escape the root directory via path traversal.
static bool is_path_within_root(const std::string& file_path, const boost::filesystem::path& root)
{
if (file_path.empty())
return false;
boost::filesystem::path p(file_path);
if (p.is_absolute())
return false;
// Reject any path component that is ".."
for (const auto& component : p) {
if (component == "..")
return false;
}
// Resolve the full path and verify it starts with the canonical root (also catches symlink escapes)
try {
boost::filesystem::path full_path = root / p;
boost::filesystem::path canonical_root = boost::filesystem::weakly_canonical(root);
boost::filesystem::path canonical_full = boost::filesystem::weakly_canonical(full_path);
auto root_str = canonical_root.string();
auto full_str = canonical_full.string();
if (full_str.length() < root_str.length())
return false;
if (full_str.compare(0, root_str.length(), root_str) != 0)
return false;
// Ensure it's a proper prefix (not just a substring of a longer directory name)
if (full_str.length() > root_str.length() &&
full_str[root_str.length()] != boost::filesystem::path::preferred_separator)
return false;
} catch (const boost::filesystem::filesystem_error&) {
return false;
}
return true;
}
// VERSION NUMBERS
// 0 : .3mf, files saved by older slic3r or other applications. No version definition in them.
// 1 : Introduction of 3mf versioning. No other change in data saved into 3mf files.

View File

@@ -1614,6 +1614,12 @@ PresetsConfigSubstitutions PresetBundle::import_presets(std::vector<std::string>
metadata.id = to_string(uuid);
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " bundle_id was empty, so generating a UUID: " << metadata.id;
}
if (has_bundle_structure && !is_path_within_root(metadata.id, user_folder / user_id / PRESET_LOCAL_DIR)) {
BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << " bundle id escapes the bundle directory, not importing: " << metadata.id;
fclose(zipFile);
fs::remove_all(temp_folder, ec);
continue;
}
// Build bundle directory path based on whether bundle_structure.json was present
fs::path bundle_base_dir;
@@ -1636,11 +1642,15 @@ PresetsConfigSubstitutions PresetBundle::import_presets(std::vector<std::string>
if (status) {
std::string file_name = file_stat.m_filename;
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " From zip file: " << file << ". Read file name: " << file_stat.m_filename;
size_t index = file_name.find_last_of('/');
size_t index = file_name.find_last_of("/\\");
if (std::string::npos != index) {
file_name = file_name.substr(index + 1);
}
if (BUNDLE_STRUCTURE_JSON_NAME == file_name) continue;
if (!is_path_within_root(file_name, temp_folder)) {
BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << " zip entry escapes the temp directory, skipping: " << file_stat.m_filename;
continue;
}
// create target file path
std::string target_file_path = boost::filesystem::path(temp_folder / file_name).make_preferred().string();
@@ -1729,6 +1739,10 @@ bool PresetBundle::import_json_presets(PresetsConfigSubstitutions & s
BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << " Preset type is unknown, not loading: " << name;
return false;
}
if (!is_path_within_root(name, fs::path(collection->m_dir_path))) {
BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << " Preset name escapes the preset directory, not loading: " << name;
return false;
}
const PresetOrigin load_origin = detect_origin_from_path(boost::filesystem::path(bundle_dir));
const std::string preset_name = get_preset_canonical_name(name, load_origin);

View File

@@ -255,6 +255,10 @@ 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.
// 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);
// Orca: custom protocal support utils
inline bool is_orca_open(const std::string& url) { return boost::starts_with(url, "orcaslicer://open"); }

View File

@@ -1088,6 +1088,30 @@ bool is_json_file(const std::string& path)
return boost::iends_with(path, ".json");
}
bool is_path_within_root(const std::string &rel_path, const boost::filesystem::path &root)
{
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;
for (size_t start = 0; start <= rel_path.size();) {
size_t end = start;
while (end < rel_path.size() && !is_separator(rel_path[end]))
++end;
if (rel_path.compare(start, end - start, "..") == 0)
return false;
start = end + 1;
}
// Resolve against the canonical root so a symlink inside it cannot lead back out.
try {
const std::string root_str = boost::filesystem::weakly_canonical(root).string();
const std::string full_str = boost::filesystem::weakly_canonical(root / rel_path).string();
return full_str.compare(0, root_str.size(), root_str) == 0 &&
(full_str.size() == root_str.size() || full_str[root_str.size()] == boost::filesystem::path::preferred_separator);
} catch (const boost::filesystem::filesystem_error &) {
return false;
}
}
bool is_img_file(const std::string &path)
{
return boost::iends_with(path, ".png") || boost::iends_with(path, ".svg");

View File

@@ -6,6 +6,8 @@
#include "libslic3r/PresetBundle.hpp"
#include "libslic3r/AppConfig.hpp"
#include "libslic3r/Utils.hpp"
#include "libslic3r/miniz_extension.hpp"
#include "test_utils.hpp"
@@ -1406,3 +1408,91 @@ TEST_CASE("Sizing down to the nozzle count plus mixes is what eats the mixed tai
CHECK(bundle.project_config.option<ConfigOptionStrings>("filament_mixed_components")->values[5] == "1,2");
}
}
namespace {
// data_dir() is a process-wide global that import_presets extracts into; scope it to the test.
struct ScopedDataDir
{
std::string previous = data_dir();
explicit ScopedDataDir(const fs::path &dir) { set_data_dir(dir.string()); }
~ScopedDataDir() { set_data_dir(previous); }
};
std::string read_file(const fs::path &file)
{
std::ifstream in(file.string(), std::ios::binary);
return std::string(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
}
void write_zip(const fs::path &zip_file, const std::vector<std::pair<std::string, std::string>> &entries)
{
mz_zip_archive zip;
mz_zip_zero_struct(&zip);
REQUIRE(open_zip_writer(&zip, zip_file.string()));
for (const auto &[name, content] : entries)
REQUIRE(mz_zip_writer_add_mem(&zip, name.c_str(), content.data(), content.size(), MZ_DEFAULT_COMPRESSION));
REQUIRE(mz_zip_writer_finalize_archive(&zip));
REQUIRE(close_zip_writer(&zip));
}
bool any_filename_contains(const fs::path &root, const std::string &needle)
{
for (fs::recursive_directory_iterator it(root), end; it != end; ++it)
if (it->path().filename().string().find(needle) != std::string::npos)
return true;
return false;
}
} // namespace
TEST_CASE("Config import confines zip entries, preset names and bundle ids to the preset directory", "[Preset][Bundle][Regression]")
{
ScopedTemporaryDir temp_dir;
const fs::path data_root = temp_dir.path() / "datadir";
const fs::path src_dir = temp_dir.path() / "src";
fs::create_directories(src_dir);
ScopedDataDir scoped_data_dir(data_root);
PresetBundle bundle;
AppConfig app_config;
const auto confirm = [](std::string const &) { return 1; };
const auto import = [&](const fs::path &file) {
std::vector<std::string> files{file.string()};
bundle.import_presets(files, confirm, ForwardCompatibilitySubstitutionRule::Disable, app_config);
return files;
};
const fs::path good_file = src_dir / "Good.json";
write_print_preset(bundle.prints.default_preset().config, good_file, "Good");
const std::string good_json = read_file(good_file);
// Four levels up from where import_presets writes (<datadir>/user/default/temp) is temp_dir
// itself, so anything that escapes lands where the scan below can see it.
const std::string up = "../../../../";
const std::string up_win = "..\\..\\..\\..\\";
SECTION("zip entry names with either separator are reduced to a basename") {
const fs::path zip = src_dir / "bundle.zip";
write_zip(zip, {{up + "zip-escape.json", "{}"}, {up_win + "zip-escape.json", "{}"}, {"presets/Good.json", good_json}});
import(zip);
CHECK(bundle.prints.find_preset("Good") != nullptr);
CHECK_FALSE(any_filename_contains(temp_dir.path(), "zip-escape"));
}
SECTION("a preset name that walks out of the preset directory is rejected") {
for (const std::string &name : {up + "name-escape", up_win + "name-escape"}) {
const fs::path file = src_dir / "escape.json";
write_print_preset(bundle.prints.default_preset().config, file, name);
CHECK(import(file).empty());
CHECK_FALSE(any_filename_contains(temp_dir.path(), "name-escape"));
}
}
SECTION("a bundle id that walks out of the bundle directory is rejected") {
const fs::path zip = src_dir / "bundle.zip";
write_zip(zip, {{BUNDLE_STRUCTURE_JSON_NAME, "{\"id\": \"" + up + "bundle-escape\"}"}, {"Good.json", good_json}});
CHECK(import(zip).empty());
CHECK_FALSE(any_filename_contains(temp_dir.path(), "bundle-escape"));
}
}