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.
Validate Plugin Symlink Targets Before Replacing Existing Files
2026-09-29 11:51:24 +00:00 · 2026-09-29 12:13:59 +08:00 · 2026-09-28 16:38:14 +08:00 · 2026-09-28 16:12:30 +08:00 · 2026-09-28 05:59:41 +08:00
11 changed files with 453 additions and 270 deletions
@@ -256,10 +256,13 @@ 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);
+// True if a symlink stored at link_rel_path (relative to root) with this target stays inside root: the target
+// must be relative, and joined to the link's directory it must pass is_path_within_root.
+extern bool is_symlink_target_within_root(const std::string &link_rel_path, const std::string &target, 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"); }
@@ -285,21 +288,6 @@ inline std::string sanitize_filename(const std::string &filename){
    const std::regex special_chars("[/\\\\:*?\"<>|]");
    return std::regex_replace(filename, special_chars, "_");
 }
-// Reduce an untrusted, possibly path-qualified name to a single sanitized file name.
-// Returns an empty string when nothing usable remains.
-inline std::string sanitize_file_basename(const std::string &name){
-    const size_t sep = name.find_last_of("/\\");
-    const std::string base = sanitize_filename(sep == std::string::npos ? name : name.substr(sep + 1));
-    // Names made only of dots and spaces refer to the folder or its parent, or are stripped to nothing on Windows.
-    return base.find_first_not_of(". ") == std::string::npos ? std::string() : base;
-}
-// Marker file a download of this process writes to before it is renamed to filename.
-boost::filesystem::path download_marker_path(const boost::filesystem::path &dest_folder, const std::string &filename);
-// Finds a sanitized variant of filename, "name(N).ext" if needed, that neither an entry of dest_folder
-// nor the download marker of another download uses. The marker at ignored_marker does not count.
-// Returns true and the name in result, or false and the last name tried.
-bool find_unused_filename(const boost::filesystem::path &dest_folder, const std::string &filename,
-                          const boost::filesystem::path &ignored_marker, std::string &result);
 // File path / name / extension splitting utilities, working with UTF-8,
 // to be published to Perl.
 namespace PerlUtils {
@@ -4,6 +4,9 @@
 #include "miniz_extension.hpp"
 #include "Utils.hpp"

+#include <boost/filesystem.hpp>
+#include <boost/log/trivial.hpp>
+
 #if defined(_MSC_VER) || defined(__MINGW64__)
 #include "boost/nowide/cstdio.hpp"
 #endif
@@ -115,6 +118,66 @@ 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;
+            }
+            // Replace a symlink at the destination rather than writing through it.
+            const boost::filesystem::path dest_path(dest_file);
+            if (boost::filesystem::is_symlink(boost::filesystem::symlink_status(dest_path)))
+                boost::filesystem::remove(dest_path);
+            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);
@@ -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:
@@ -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]))
@@ -1103,7 +1106,10 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
    }
    // 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();
+        std::string root_str = boost::filesystem::weakly_canonical(root).string();
+        // A trailing separator on root would otherwise fail the prefix match below for every path.
+        while (!root_str.empty() && (root_str.back() == '/' || root_str.back() == boost::filesystem::path::preferred_separator))
+            root_str.pop_back();
        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);
@@ -1112,6 +1118,15 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
    }
 }

+bool is_symlink_target_within_root(const std::string &link_rel_path, const std::string &target, const boost::filesystem::path &root)
+{
+    if (target.empty() || target.front() == '/' || target.front() == '\\' || (target.size() > 1 && target[1] == ':'))
+        return false;
+    // A relative target without ".." only descends from the link's directory, so no chain of such links can leave root.
+    const size_t sep = link_rel_path.find_last_of("/\\");
+    return is_path_within_root((sep == std::string::npos ? std::string() : link_rel_path.substr(0, sep + 1)) + target, root);
+}
+
 bool is_img_file(const std::string &path)
 {
 	return boost::iends_with(path, ".png") || boost::iends_with(path, ".svg");
@@ -1321,31 +1336,6 @@ unsigned get_current_pid()
 #endif
 }

-boost::filesystem::path download_marker_path(const boost::filesystem::path &dest_folder, const std::string &filename)
-{
-    return dest_folder / (filename + "." + std::to_string(get_current_pid()) + ".download");
-}
-
-bool find_unused_filename(const boost::filesystem::path &dest_folder, const std::string &filename,
-                          const boost::filesystem::path &ignored_marker, std::string &result)
-{
-    // Probe the name that will be written, so a name the sanitizing maps onto an existing file is versioned too.
-    const std::string sanitized = sanitize_filename(filename);
-    const std::string extension = boost::filesystem::path(sanitized).extension().string();
-    const std::string stem      = sanitized.substr(0, sanitized.size() - extension.size());
-    auto is_used = [&](const std::string &name) {
-        const boost::filesystem::path marker = download_marker_path(dest_folder, name);
-        return boost::filesystem::exists(dest_folder / name) || (marker != ignored_marker && boost::filesystem::exists(marker));
-    };
-    result = sanitized;
-    for (size_t version = 1; is_used(result); ++version) {
-        if (version > 999)
-            return false;
-        result = stem + "(" + std::to_string(version) + ")" + extension;
-    }
-    return true;
-}
-
 std::string per_user_temp_id()
 {
 #ifdef WIN32
@@ -71,6 +71,17 @@ bool FileGet::is_subdomain(const std::string& url, const std::string& domain)
 	return false;
 }

+namespace {
+unsigned get_current_pid()
+{
+#ifdef WIN32
+	return GetCurrentProcessId();
+#else
+	return ::getpid();
+#endif
+}
+}
+
 // int = DOWNLOAD ID; string = file path
 wxDEFINE_EVENT(EVT_DWNLDR_FILE_COMPLETE, wxCommandEvent);
 // int = DOWNLOAD ID; string = error msg
@@ -133,10 +144,25 @@ void FileGet::priv::get_perform()
 	std::string extension;
 	if (m_written == 0)
 	{
-		std::string final_filename;
-		bool found = false;
+		boost::filesystem::path dest_path = m_dest_folder / m_filename;
+		extension = dest_path.extension().string();
+		std::string just_filename = m_filename.substr(0, m_filename.size() - extension.size());
+		std::string final_filename = just_filename;
+        // Find unsed filename 
 		try {
-			found = find_unused_filename(m_dest_folder, m_filename, m_tmp_path, final_filename);
+			size_t version = 0;
+			while (boost::filesystem::exists(m_dest_folder / (final_filename + extension)) || boost::filesystem::exists(m_dest_folder / (final_filename + extension + "." + std::to_string(get_current_pid()) + ".download")))
+			{
+				++version;
+				if (version > 999) {
+					wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR);
+					evt->SetString(GUI::format_wxstr(L"Failed to find suitable filename. Last name: %1%." , (m_dest_folder / (final_filename + extension)).string()));
+					evt->SetInt(m_id);
+					m_evt_handler->QueueEvent(evt);
+					return;
+				}
+				final_filename = GUI::format("%1%(%2%)", just_filename, std::to_string(version));
+			}
 		} catch (const boost::filesystem::filesystem_error& e)
 		{
 			wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR);
@@ -145,18 +171,10 @@ void FileGet::priv::get_perform()
 			m_evt_handler->QueueEvent(evt);
 			return;
 		}
-		if (!found) {
-			wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR);
-			evt->SetString(GUI::format_wxstr(L"Failed to find suitable filename. Last name: %1%." , (m_dest_folder / final_filename).string()));
-			evt->SetInt(m_id);
-			m_evt_handler->QueueEvent(evt);
-			return;
-		}

-        m_filename = final_filename;
-		extension = boost::filesystem::path(m_filename).extension().string();
+        m_filename = sanitize_filename(final_filename + extension);

-        m_tmp_path = download_marker_path(m_dest_folder, m_filename);
+        m_tmp_path = m_dest_folder / (m_filename + "." + std::to_string(get_current_pid()) + ".download");

 		wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE);
 		evt->SetString(boost::nowide::widen(m_filename));
@@ -203,32 +221,7 @@ void FileGet::priv::get_perform()
 			if(dest_path.empty()) {
 				std::string filename = extract_remote_filename(header);
 				if (!filename.empty()) {
-					// The name comes from the server: keep it inside the destination folder and never
-					// replace an existing file. Keep the current name if nothing usable remains.
-					filename = sanitize_file_basename(filename);
-					std::string unused;
-					try {
-						if (filename.empty() || !find_unused_filename(m_dest_folder, filename, m_tmp_path, unused))
-							unused.clear();
-					} catch (const boost::filesystem::filesystem_error&) {
-						unused.clear();
-					}
-					const boost::filesystem::path tmp_path = unused.empty() ? m_tmp_path : download_marker_path(m_dest_folder, unused);
-					if (tmp_path != m_tmp_path) {
-						// Move the marker to the adopted name so that other downloads see the name as taken.
-						// Only before anything is written, so that no downloaded data has to be carried over.
-						FILE* tmp_file = m_written == 0 ? fopen(wxString(tmp_path.wstring()).c_str(), "wb") : nullptr;
-						if (tmp_file != nullptr) {
-							fclose(file);
-							boost::system::error_code ec;
-							boost::filesystem::remove(m_tmp_path, ec);
-							file = tmp_file;
-							m_tmp_path = tmp_path;
-						} else
-							unused.clear();
-					}
-					if (!unused.empty())
-						m_filename = unused;
+					m_filename = filename;
 					dest_path = m_dest_folder / m_filename;
 					wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE);
 					evt->SetString(boost::nowide::widen(m_filename));
@@ -334,18 +327,6 @@ void FileGet::priv::get_perform()
                    m_evt_handler->QueueEvent(evt);
                }
 				fclose(file);
-				// Another file may have taken the name while downloading.
-				if (!dest_path.empty() && boost::filesystem::exists(dest_path)) {
-					std::string unused;
-					if (!find_unused_filename(m_dest_folder, m_filename, m_tmp_path, unused))
-						throw std::runtime_error("No unused file name.");
-					m_filename = unused;
-					dest_path = m_dest_folder / m_filename;
-					wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE);
-					evt->SetString(boost::nowide::widen(m_filename));
-					evt->SetInt(m_id);
-					m_evt_handler->QueueEvent(evt);
-				}
 				boost::filesystem::rename(m_tmp_path, dest_path);
 			}
 			catch (const std::exception& /*e*/)
@@ -1512,11 +1512,33 @@ 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());
+#ifndef WIN32
+                // Validate a symlink's target before anything at the destination is replaced.
+                const bool is_link = S_ISLNK(stat.m_external_attr >> 16);
+                std::string link;
+                if (is_link) {
+                    link.assign(stat.m_uncomp_size, 0);
+                    if (!mz_zip_reader_extract_to_mem(&archive, stat.m_file_index, link.data(), stat.m_uncomp_size, 0) ||
+                        !is_symlink_target_within_root(dest_file, link, plugin_folder)) {
+                        BOOST_LOG_TRIVIAL(error) << "[install_plugin] link " << dest_file << " -> " << link << " is unreadable or resolves outside " << plugin_folder.string();
+                        close_zip_reader(&archive);
+                        if (pro_fn) { pro_fn(InstallStatusUnzipFailed, 0, cancel); }
+                        return InstallStatusUnzipFailed;
+                    }
+                }
+#endif
                try {
-                    if (fs::exists(dest_path)) {
+                    boost::filesystem::create_directories(dest_path.parent_path());
+                    // symlink_status so that an existing symlink, dangling or not, is replaced rather than written through.
+                    if (fs::exists(fs::symlink_status(dest_path))) {
                        boost::system::error_code ec;
                        fs::remove(dest_path, ec);
                        if (ec) {
@@ -1544,9 +1566,8 @@ int GUI_App::install_plugin(std::string name, std::string package_name, InstallP
                    }
                    mz_bool res = 0;
 #ifndef WIN32
-                    if (S_ISLNK(stat.m_external_attr >> 16)) {
-                        std::string link(stat.m_uncomp_size + 1, 0);
-                        res = mz_zip_reader_extract_to_mem(&archive, stat.m_file_index, link.data(), stat.m_uncomp_size, 0);
+                    if (is_link) {
+                        res = 1;
                        try {
                            boost::filesystem::create_symlink(link, dest_path);
                        } catch (const std::exception &e) {
@@ -15681,11 +15681,6 @@ void Plater::import_model_id(wxString download_info)
        //wxString sError = error.what();
    }

-    // The name comes from the link: reduce it to a plain file name inside the download folder.
-    filename = from_u8(sanitize_file_basename(into_u8(filename)));
-    if (filename.empty())
-        filename = "untitled.3mf";
-
    bool download_ok = false;
    int retry_count = 0;
    const int max_retries = 3;
@@ -15727,28 +15722,51 @@ void Plater::import_model_id(wxString download_info)

        msg = _L("Preparing 3MF file...");

+        //gets the number of files with the same name
+        std::vector<wxString>   vecFiles;
+        bool                    is_already_exist = false;
+
+
        target_path = fs::path(wxGetApp().app_config->get("download_path"));

-        //check file suffix
-        wxString extension = fs::path(filename.wx_str()).extension().c_str();
-        if (!extension.Contains(".3mf")) {
-            msg = _L("Download failed; unknown file format.");
-            return;
+        try
+        {
+            vecFiles.clear();
+            wxString extension = fs::path(filename.wx_str()).extension().c_str();
+
+
+            //check file suffix
+            if (!extension.Contains(".3mf")) {
+                msg = _L("Download failed; unknown file format.");
+                return;
+            }
+
+            auto name = filename.substr(0, filename.length() - extension.length() - 1);
+
+            for (const auto& iter : boost::filesystem::directory_iterator(target_path))
+            {
+                if (boost::filesystem::is_directory(iter.path()))
+                    continue;
+
+                wxString sFile = iter.path().filename().string().c_str();
+                if (strstr(sFile.c_str(), name.c_str()) != NULL) {
+                    vecFiles.push_back(sFile);
+                }
+
+                if (sFile == filename) is_already_exist = true;
+            }
+        }
+        catch (const std::exception&)
+        {
+            //wxString sError = error.what();
        }

-        //never replace an existing file
-        std::string unused_filename;
-        try {
-            if (!find_unused_filename(target_path, into_u8(filename), {}, unused_filename))
-                unused_filename.clear();
-        } catch (const std::exception&) {
-            unused_filename.clear();
+        //update filename
+        if (is_already_exist && vecFiles.size() >= 1) {
+            wxString extension = fs::path(filename.wx_str()).extension().c_str();
+            wxString name = filename.substr(0, filename.length() - extension.length());
+            filename = wxString::Format("%s(%d)%s", name, vecFiles.size() + 1, extension).ToStdString();
        }
-        if (unused_filename.empty()) {
-            msg = _L("Importing to Orca Slicer failed. Please download the file and manually import it.");
-            return;
-        }
-        filename = from_u8(unused_filename);


        msg = _L("Downloading project...");
@@ -15760,6 +15778,10 @@ void Plater::import_model_id(wxString download_info)
        boost::uuids::uuid uuid = boost::uuids::random_generator()();
        std::string unique = to_string(uuid).substr(0, 6);

+        if (filename.empty()) {
+            filename = "untitled.3mf";
+        }
+
        //target_path /= (boost::format("%1%_%2%.3mf") % filename % unique).str();
        target_path /= fs::path(filename.wc_str());

@@ -15808,26 +15830,13 @@ void Plater::import_model_id(wxString download_info)
                        cont = false;
                    }
                })
-                .on_complete([&cont, &download_ok, &msg, tmp_path, &target_path](std::string body, unsigned /* http_status */) {
+                .on_complete([&cont, &download_ok, tmp_path, target_path](std::string body, unsigned /* http_status */) {
                        fs::fstream file(tmp_path, std::ios::out | std::ios::binary | std::ios::trunc);
                        file.write(body.c_str(), body.size());
                        file.close();
+                        fs::rename(tmp_path, target_path);
                        cont = false;
-                        try {
-                            // Another file may have taken the name while downloading.
-                            std::string unused_filename;
-                            if (find_unused_filename(target_path.parent_path(), target_path.filename().string(), {}, unused_filename)) {
-                                target_path = target_path.parent_path() / unused_filename;
-                                fs::rename(tmp_path, target_path);
-                                download_ok = true;
-                                return;
-                            }
-                        } catch (const std::exception &e) {
-                            BOOST_LOG_TRIVIAL(error) << "import_model_id: failed to move the download into place: " << e.what();
-                        }
-                        boost::system::error_code ec;
-                        fs::remove(tmp_path, ec);
-                        msg = _L("Importing to Orca Slicer failed. Please download the file and manually import it.");
+                        download_ok = true;
                }).perform_sync();

                // for break while
@@ -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 "<<file_path;
-        return false;
-    }
-
-    mz_uint num_entries = mz_zip_reader_get_num_files(&archive);
-
-    mz_zip_archive_file_stat stat;
-    // we first loop the entries to read from the archive the .amf file only, in order to extract the version from it
-    for (mz_uint i = 0; i < num_entries; ++i)
-    {
-        if (mz_zip_reader_file_stat(&archive, i, &stat))
-        {
-            std::string dest_file = parent_path+"/"+stat.m_filename;
-            if (stat.m_is_directory) {
-                fs::path dest_path(dest_file);
-                if (!fs::exists(dest_path))
-                    fs::create_directories(dest_path);
-				continue;
-            }
-            else if (stat.m_uncomp_size == 0) {
-                BOOST_LOG_TRIVIAL(warning) << "[Orca Updater]Unzip: invalid size for file "<<stat.m_filename;
-                continue;
-            }
-            try
-            {
-                res = mz_zip_reader_extract_to_file(&archive, stat.m_file_index, dest_file.c_str(), 0);
-                if (!res) {
-                    BOOST_LOG_TRIVIAL(error) << "[Orca Updater]extract file "<<stat.m_filename<<" to dest "<<dest_file<<" failed";
-                    close_zip_reader(&archive);
-                    return res;
-                }
-                BOOST_LOG_TRIVIAL(info) << "[Orca Updater]successfully extract file " << stat.m_file_index << " to "<<dest_file;
-            }
-            catch (const std::exception& e)
-            {
-                // ensure the zip archive is closed and rethrow the exception
-                close_zip_reader(&archive);
-                BOOST_LOG_TRIVIAL(error) << "[Orca Updater]Archive read exception:"<<e.what();
-                return false;
-            }
-        }
-        else {
-            BOOST_LOG_TRIVIAL(warning) << "[Orca Updater]Unzip: read file stat failed";
-        }
-    }
-    close_zip_reader(&archive);
-
-	return true;
+    const std::string parent_path = (!dest_path.empty() ? dest_path : source_path.parent_path()).string();
+    return extract_archive_confined(source_path.string(), parent_path);
 }

 // Remove a leftover partial archive for the vendor about to be synchronized.
@@ -40,6 +40,7 @@ add_executable(${_TEST_NAME}_tests
    test_lay_on_face.cpp
    test_model.cpp
    test_utils.cpp
+    test_miniz_extension.cpp
    test_timeutils.cpp
    test_voronoi.cpp
    test_wipe_tower_estimate.cpp
@@ -0,0 +1,194 @@
+#include <catch2/catch_all.hpp>
+
+#include "libslic3r/miniz_extension.hpp"
+
+#include "test_utils.hpp"
+
+#include <boost/filesystem.hpp>
+
+#include <algorithm>
+#include <fstream>
+#include <iterator>
+#include <string>
+#include <utility>
+#include <vector>
+
+using namespace Slic3r;
+namespace fs = boost::filesystem;
+
+namespace {
+
+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));
+}
+
+// miniz refuses to write a name starting with '/', so write a placeholder of the same length and patch it in place.
+void rename_entry(const fs::path &zip_file, const std::string &from, const std::string &to)
+{
+    REQUIRE(from.size() == to.size());
+    std::string bytes;
+    {
+        std::ifstream in(zip_file.string(), std::ios::binary);
+        bytes.assign(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
+    }
+    size_t count = 0;
+    for (size_t pos = bytes.find(from); pos != std::string::npos; pos = bytes.find(from, pos + to.size()), ++count)
+        bytes.replace(pos, from.size(), to);
+    // Once in the local header and once in the central directory.
+    REQUIRE(count == 2);
+    std::ofstream out(zip_file.string(), std::ios::binary | std::ios::trunc);
+    out << bytes;
+}
+
+std::vector<std::string> list_dir(const fs::path &dir)
+{
+    std::vector<std::string> names;
+    for (const fs::directory_entry &entry : fs::directory_iterator(dir))
+        names.push_back(entry.path().filename().string());
+    std::sort(names.begin(), names.end());
+    return names;
+}
+
+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>());
+}
+
+} // 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"),
+                                                std::string("C:escape.txt"), std::string("\\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));
+}
+
+TEST_CASE("Confined extraction rejects an archive with an absolute entry name", "[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 absolute    = (tmp.path() / "escape.txt").generic_string();
+    const std::string placeholder = "#" + absolute.substr(1);
+    write_zip(zip_file, {{"normal.json", "{}"}, {placeholder, "escaped"}});
+    rename_entry(zip_file, placeholder, absolute);
+
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction rejects a directory 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);
+    write_zip(zip_file, {{"vendor/", ""}, {"../outside/", ""}});
+
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "outside"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction validates zero-size entries like any other", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    SECTION("an empty file inside the target does not fail the archive") {
+        write_zip(zip_file, {{"empty.json", ""}, {"vendor.json", "{}"}});
+        CHECK(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK(read_file(target / "vendor.json") == "{}");
+    }
+    SECTION("an empty file outside the target rejects the archive") {
+        write_zip(zip_file, {{"vendor.json", "{}"}, {"../escape.txt", ""}});
+        CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+        CHECK(fs::is_empty(target));
+    }
+}
+
+TEST_CASE("Confined extraction writes nothing outside the target for Windows-specific name forms", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    // Windows strips trailing dots and spaces and maps device names; whether these extract depends on the
+    // platform, but none of them may land beside the target.
+    const std::string name = GENERATE(std::string("name."), std::string("name "), std::string("..."), std::string(".. "),
+                                      std::string(".. /escape.txt"), std::string(".../escape.txt"), std::string("CON"),
+                                      std::string("sub/NUL.txt"), std::string("C:escape.txt"));
+    write_zip(zip_file, {{name, "payload"}});
+
+    CAPTURE(name);
+    extract_archive_confined(zip_file.string(), target.string());
+    CHECK(list_dir(tmp.path()) == std::vector<std::string>{"bundle.zip", "cache"});
+}
+
+#ifndef _WIN32
+TEST_CASE("Confined extraction replaces a symlink at the destination instead of writing through it", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    const fs::path     outside  = tmp.path() / "outside";
+    fs::create_directories(target);
+    fs::create_directories(outside);
+    write_zip(zip_file, {{"vendor.json", "{\"a\":1}"}});
+
+    SECTION("a dangling symlink") {
+        fs::create_symlink(outside / "vendor.json", target / "vendor.json");
+        CHECK(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK_FALSE(fs::exists(outside / "vendor.json"));
+        CHECK_FALSE(fs::is_symlink(fs::symlink_status(target / "vendor.json")));
+        CHECK(read_file(target / "vendor.json") == "{\"a\":1}");
+    }
+    SECTION("a symlink to an existing file") {
+        { std::ofstream((outside / "vendor.json").string()) << "original"; }
+        fs::create_symlink(outside / "vendor.json", target / "vendor.json");
+        extract_archive_confined(zip_file.string(), target.string());
+        CHECK(read_file(outside / "vendor.json") == "original");
+    }
+}
+#endif
@@ -153,93 +153,81 @@ TEST_CASE("resolve_cli_input_path leaves inputs that must not be completed uncha
    }
 }

-TEST_CASE("sanitize_file_basename keeps only a plain file name from an untrusted name", "[Utils]") {
-    const std::string unicode = "\xe6\xa8\xa1\xe5\x9e\x8b \xc3\xa9t\xc3\xa9.3mf"; // UTF-8 CJK and accented Latin
-    const auto [input, expected] = GENERATE_COPY(table<std::string, std::string>({
-        {"normal.3mf", "normal.3mf"},
-        {"../../x.3mf", "x.3mf"},
-        {"..\\..\\x.3mf", "x.3mf"},
-        {"C:\\x.3mf", "x.3mf"},
-        {"C:x.3mf", "C_x.3mf"},
-        {"/etc/x", "x"},
-        {"a/b\\c.gcode", "c.gcode"},
-        {"x:stream", "x_stream"}, // no NTFS alternate data stream
-        {"x.", "x."},
-        {".3mf", ".3mf"},
-        {unicode, unicode},
-    }));
-    CAPTURE(input);
-    CHECK(sanitize_file_basename(input) == expected);
+TEST_CASE("is_path_within_root accepts a root given with a trailing separator", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const std::string root = tmp.path().string();
+    const std::string with_separator = GENERATE_COPY(root + "/", root + std::string(1, static_cast<char>(boost::filesystem::path::preferred_separator)));
+
+    CAPTURE(with_separator);
+    CHECK(is_path_within_root("vendor.json", with_separator));
+    CHECK(is_path_within_root("vendor/machine/printer.json", with_separator));
+    CHECK_FALSE(is_path_within_root("../vendor.json", with_separator));
 }

-TEST_CASE("sanitize_file_basename rejects names that do not name a file", "[Utils]") {
-    const std::string input = GENERATE(as<std::string>{}, "", ".", "..", "../..", "dir/", "..\\", " ", ". .", "...");
-    CAPTURE(input);
-    CHECK(sanitize_file_basename(input).empty());
+TEST_CASE("is_path_within_root treats Windows-specific name forms the same on every platform", "[utils]") {
+    ScopedTemporaryDir tmp;
+
+    SECTION("names ending in dots or spaces stay inside the root") {
+        const std::string name = GENERATE(std::string("name."), std::string("name "), std::string("dir./file.json"), std::string("dir /file.json"));
+        CAPTURE(name);
+        CHECK(is_path_within_root(name, tmp.path()));
+    }
+    SECTION("drive-relative names are rejected") {
+        const std::string name = GENERATE(std::string("C:x"), std::string("c:x/y.json"), std::string("C:"));
+        CAPTURE(name);
+        CHECK_FALSE(is_path_within_root(name, tmp.path()));
+    }
 }

-namespace {
-void touch(const boost::filesystem::path &path) { std::ofstream(path.string()) << "existing"; }
-std::string file_contents(const boost::filesystem::path &path)
-{
-    std::ifstream file(path.string());
-    return std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>());
-}
-} // namespace
-
-TEST_CASE("find_unused_filename keeps a name nothing uses", "[Utils]") {
-    ScopedTemporaryDir dir;
-    std::string name;
-    REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name));
-    CHECK(name == "model.3mf");
+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("find_unused_filename versions a name an existing file uses", "[Utils]") {
-    ScopedTemporaryDir dir;
-    touch(dir.path() / "model.3mf");
-    std::string name;
-    REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name));
-    CHECK(name == "model(1).3mf");
+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")),
+                                         std::make_pair(std::string("Foo.framework/Foo"), std::string("Versions/Current/Foo")),
+                                         std::make_pair(std::string("libfoo.so"), std::string("libfoo.so.1")),
+                                         std::make_pair(std::string("a/b/link"), std::string("c/d")));
+    CAPTURE(link, target);
+    CHECK(is_symlink_target_within_root(link, target, tmp.path()));
 }

-TEST_CASE("find_unused_filename versions a name that maps onto an existing file once sanitized", "[Utils]") {
-    ScopedTemporaryDir dir;
-    touch(dir.path() / "my_model.3mf");
-    const std::string input = GENERATE(as<std::string>{}, "my?model.3mf", "my:model.3mf", "my*model.3mf");
-    CAPTURE(input);
-    std::string name;
-    REQUIRE(find_unused_filename(dir.path(), input, {}, name));
-    CHECK(name == "my_model(1).3mf");
-    CHECK(file_contents(dir.path() / "my_model.3mf") == "existing");
+TEST_CASE("is_symlink_target_within_root rejects absolute targets and targets that climb out", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const std::string outside = (tmp.path().parent_path() / "outside").generic_string();
+    const auto [link, target] = GENERATE_COPY(std::make_pair(std::string("sub/link"), outside),
+                                              std::make_pair(std::string("sub/link"), std::string("/etc/passwd")),
+                                              std::make_pair(std::string("sub/link"), std::string("\\outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("C:/outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("C:outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("")),
+                                              std::make_pair(std::string("link"), std::string("..")),
+                                              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")),
+                                              // 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()));
 }

-TEST_CASE("find_unused_filename treats the marker of another download as used", "[Utils]") {
-    ScopedTemporaryDir dir;
-    touch(download_marker_path(dir.path(), "model.3mf"));
-    std::string name;
-    REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name));
-    CHECK(name == "model(1).3mf");
-}
+#ifndef _WIN32
+TEST_CASE("is_symlink_target_within_root rejects a target that passes through a symlink leading out", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const boost::filesystem::path root    = tmp.path() / "root";
+    const boost::filesystem::path outside = tmp.path() / "outside";
+    boost::filesystem::create_directories(root);
+    boost::filesystem::create_directories(outside);
+    boost::filesystem::create_symlink(outside, root / "out");

-TEST_CASE("find_unused_filename ignores the marker of the download asking", "[Utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path own_marker = download_marker_path(dir.path(), "model.3mf");
-    touch(own_marker);
-    std::string name;
-    REQUIRE(find_unused_filename(dir.path(), "model.3mf", own_marker, name));
-    CHECK(name == "model.3mf");
-}
-
-TEST_CASE("find_unused_filename gives up after 999 versions", "[Utils]") {
-    ScopedTemporaryDir dir;
-    touch(dir.path() / "model.3mf");
-    for (int version = 1; version < 999; ++version)
-        touch(dir.path() / ("model(" + std::to_string(version) + ").3mf"));
-    std::string name;
-    REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name));
-    CHECK(name == "model(999).3mf");
-
-    touch(dir.path() / name);
-    REQUIRE_FALSE(find_unused_filename(dir.path(), "model.3mf", {}, name));
-    CHECK(name == "model(999).3mf");
+    CHECK_FALSE(is_symlink_target_within_root("link", "out/lib.so", root));
+    CHECK(is_symlink_target_within_root("link", "in/lib.so", root));
 }
+#endif
Author	SHA1	Message	Date
Hanif Koh	74cb23465b	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.	2026-09-29 12:13:59 +08:00
Hanif Koh	4b8b9abb5e	Validate Plugin Symlink Targets Before Replacing Existing Files A symlink entry's target is now read and checked before anything already at its destination is removed or renamed aside, so an archive rejected for its link target leaves the installed plugin files in place.	2026-09-28 16:38:14 +08:00
Hanif Koh	365e4173e1	Harden Archive Extraction Against Symlinks The plugin installer now creates a symlink entry only when its target is relative and, joined to the link's own directory, passes is_path_within_root, via the new is_symlink_target_within_root helper. Before writing any entry it checks the destination with symlink_status, so an existing symlink, dangling or not, is replaced rather than followed, and it creates parent directories inside the existing error handling. extract_archive_confined replaces a symlink at a destination file the same way. is_path_within_root now ignores a trailing separator on the root, which previously made every path fail the check.	2026-09-28 16:12:30 +08:00
Hanif Koh	6dd8401c59	Confine Updater Archive Extraction to the Target Directory The preset updater extracted downloaded archives by appending each entry name to the cache directory, and the network plugin installer did the same for the plugin folder, without checking that the result stays inside it. Move the updater's extraction into libslic3r as extract_archive_confined, which validates every entry with is_path_within_root before writing anything and fails the whole archive if one entry resolves outside the target. The plugin installer now rejects such an entry the same way. Well formed archives extract exactly as before.	2026-09-28 05:59:41 +08:00