diff --git a/src/libslic3r/Format/bbs_3mf.cpp b/src/libslic3r/Format/bbs_3mf.cpp index b0cbb1fd50..b4f6dc1d45 100644 --- a/src/libslic3r/Format/bbs_3mf.cpp +++ b/src/libslic3r/Format/bbs_3mf.cpp @@ -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. diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index 6d4e77837a..f17fd5c768 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -1614,6 +1614,12 @@ PresetsConfigSubstitutions PresetBundle::import_presets(std::vector 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 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); diff --git a/src/libslic3r/Utils.hpp b/src/libslic3r/Utils.hpp index 55d9b716cf..797894442a 100644 --- a/src/libslic3r/Utils.hpp +++ b/src/libslic3r/Utils.hpp @@ -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"); } diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 5f4baac951..875c90f6ab 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -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"); diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 26cbf387ec..d01711000a 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -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("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(in), std::istreambuf_iterator()); +} + +void write_zip(const fs::path &zip_file, const std::vector> &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 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 (/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")); + } +}