diff --git a/src/libslic3r/miniz_extension.cpp b/src/libslic3r/miniz_extension.cpp index 53a4a8e9de..620cb6fb27 100644 --- a/src/libslic3r/miniz_extension.cpp +++ b/src/libslic3r/miniz_extension.cpp @@ -4,6 +4,9 @@ #include "miniz_extension.hpp" #include "Utils.hpp" +#include +#include + #if defined(_MSC_VER) || defined(__MINGW64__) #include "boost/nowide/cstdio.hpp" #endif @@ -115,6 +118,62 @@ std::string decode_archive_entry_path(mz_zip_archive *zip, const mz_zip_archive_ return decode_zip_unicode_path_extra_field(extra.substr(0, extra_size > 0 ? extra_size - 1 : 0), stat.m_filename); } +bool extract_archive_confined(const std::string &zip_path_utf8, const std::string &dest_dir) +{ + mz_zip_archive archive; + mz_zip_zero_struct(&archive); + + if (!open_zip_reader(&archive, zip_path_utf8)) { + BOOST_LOG_TRIVIAL(error) << "Unable to open zip reader for " << zip_path_utf8; + return false; + } + + const mz_uint num_entries = mz_zip_reader_get_num_files(&archive); + mz_zip_archive_file_stat stat; + + // Validate every entry first so an archive with a single escaping entry leaves no partial output behind. + const boost::filesystem::path root(dest_dir); + for (mz_uint i = 0; i < num_entries; ++i) { + if (mz_zip_reader_file_stat(&archive, i, &stat) && !is_path_within_root(stat.m_filename, root)) { + BOOST_LOG_TRIVIAL(error) << "Unzip: rejecting " << zip_path_utf8 << ", entry " << stat.m_filename << " resolves outside " << dest_dir; + close_zip_reader(&archive); + return false; + } + } + + for (mz_uint i = 0; i < num_entries; ++i) { + if (!mz_zip_reader_file_stat(&archive, i, &stat)) { + BOOST_LOG_TRIVIAL(warning) << "Unzip: read file stat failed"; + continue; + } + const std::string dest_file = dest_dir + "/" + stat.m_filename; + try { + if (stat.m_is_directory) { + const boost::filesystem::path dest_path(dest_file); + if (!boost::filesystem::exists(dest_path)) + boost::filesystem::create_directories(dest_path); + continue; + } + if (stat.m_uncomp_size == 0) { + BOOST_LOG_TRIVIAL(warning) << "Unzip: invalid size for file " << stat.m_filename; + continue; + } + if (!mz_zip_reader_extract_to_file(&archive, stat.m_file_index, dest_file.c_str(), 0)) { + BOOST_LOG_TRIVIAL(error) << "Unzip: extract file " << stat.m_filename << " to dest " << dest_file << " failed"; + close_zip_reader(&archive); + return false; + } + BOOST_LOG_TRIVIAL(info) << "Unzip: successfully extract file " << stat.m_file_index << " to " << dest_file; + } catch (const std::exception &e) { + close_zip_reader(&archive); + BOOST_LOG_TRIVIAL(error) << "Unzip: archive read exception: " << e.what(); + return false; + } + } + close_zip_reader(&archive); + return true; +} + MZ_Archive::MZ_Archive() { mz_zip_zero_struct(&arch); diff --git a/src/libslic3r/miniz_extension.hpp b/src/libslic3r/miniz_extension.hpp index 1a1c96689f..97aa91b93f 100644 --- a/src/libslic3r/miniz_extension.hpp +++ b/src/libslic3r/miniz_extension.hpp @@ -11,6 +11,8 @@ bool open_zip_writer(mz_zip_archive *zip, const std::string &fname_utf8); bool close_zip_reader(mz_zip_archive *zip); bool close_zip_writer(mz_zip_archive *zip); std::string decode_archive_entry_path(mz_zip_archive *zip, const mz_zip_archive_file_stat &stat); +// Extracts every entry of the archive under dest_dir. Nothing is written if any entry would resolve outside dest_dir. +bool extract_archive_confined(const std::string &zip_path_utf8, const std::string &dest_dir); class MZ_Archive { public: diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index a41001f9e0..99f0d8088a 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -1512,6 +1512,12 @@ int GUI_App::install_plugin(std::string name, std::string package_name, InstallP size_t n = mz_zip_reader_get_extra(&archive, stat.m_file_index, extra.data(), extra.size()); dest_file = decode(extra.substr(0, n), stat.m_filename); } + if (!is_path_within_root(dest_file, plugin_folder)) { + BOOST_LOG_TRIVIAL(error) << "[install_plugin] entry " << dest_file << " resolves outside " << plugin_folder.string(); + close_zip_reader(&archive); + if (pro_fn) { pro_fn(InstallStatusUnzipFailed, 0, cancel); } + return InstallStatusUnzipFailed; + } auto dest_path = plugin_folder / dest_file; boost::filesystem::create_directories(dest_path.parent_path()); std::string dest_zip_file = encode_path(dest_path.string().c_str()); diff --git a/src/slic3r/Utils/PresetUpdater.cpp b/src/slic3r/Utils/PresetUpdater.cpp index b8379697d6..5bbe779970 100644 --- a/src/slic3r/Utils/PresetUpdater.cpp +++ b/src/slic3r/Utils/PresetUpdater.cpp @@ -339,62 +339,8 @@ bool PresetUpdater::priv::get_file(const std::string &url, const fs::path &targe //BBS: refine preset update logic bool PresetUpdater::priv::extract_file(const fs::path &source_path, const fs::path &dest_path) { - bool res = true; - std::string file_path = source_path.string(); - std::string parent_path = (!dest_path.empty() ? dest_path : source_path.parent_path()).string(); - mz_zip_archive archive; - mz_zip_zero_struct(&archive); - - if (!open_zip_reader(&archive, file_path)) - { - BOOST_LOG_TRIVIAL(error) << "Unable to open zip reader for "< + +#include "libslic3r/miniz_extension.hpp" + +#include "test_utils.hpp" + +#include + +#include +#include +#include +#include +#include + +using namespace Slic3r; +namespace fs = boost::filesystem; + +namespace { + +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)); +} + +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()); +} + +} // namespace + +TEST_CASE("Confined extraction writes a well-formed archive under the target directory", "[MinizExtension]") +{ + ScopedTemporaryDir tmp; + const fs::path zip_file = tmp.path() / "bundle.zip"; + const fs::path target = tmp.path() / "cache"; + fs::create_directories(target); + write_zip(zip_file, {{"vendor/", ""}, {"vendor/machine/", ""}, {"vendor.json", "{\"a\":1}"}, {"vendor/machine/printer.json", "{\"b\":2}"}}); + + REQUIRE(extract_archive_confined(zip_file.string(), target.string())); + CHECK(fs::is_directory(target / "vendor")); + CHECK(read_file(target / "vendor.json") == "{\"a\":1}"); + CHECK(read_file(target / "vendor" / "machine" / "printer.json") == "{\"b\":2}"); +} + +TEST_CASE("Confined extraction rejects an archive with an entry outside the target directory", "[MinizExtension]") +{ + ScopedTemporaryDir tmp; + const fs::path zip_file = tmp.path() / "bundle.zip"; + const fs::path target = tmp.path() / "cache"; + fs::create_directories(target); + + const std::string escaping_entry = GENERATE(std::string("../escape.txt"), std::string("..\\escape.txt"), + std::string("sub/../../escape.txt"), std::string("C:/escape.txt")); + // The normal entry comes first so a per-entry check would already have written it. + write_zip(zip_file, {{"normal.json", "{}"}, {escaping_entry, "escaped"}}); + + CAPTURE(escaping_entry); + CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string())); + CHECK_FALSE(fs::exists(tmp.path() / "escape.txt")); + CHECK(fs::is_empty(target)); +}