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.

src/libslic3r/Format/bbs_3mf.cpp

@@ -1629,7 +1629,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
         }
         while (it != m_plater_data.end())
         {
-            if (it->first <= 0 || static_cast<size_t>(it->first) > m_plater_data.size())
+            if (it->first > m_plater_data.size())
             {
                 add_error("invalid plate index");
                 return false;
@@ -2312,7 +2312,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
         }
         while (it != m_plater_data.end())
         {
-            if (it->first <= 0 || static_cast<size_t>(it->first) > m_plater_data.size())
+            if (it->first > m_plater_data.size())
             {
                 add_error("invalid plate index");
                 return false;

src/libslic3r/Model.cpp

@@ -3702,11 +3702,7 @@ void FacetsAnnotation::set_triangle_from_string(int triangle_id, const std::stri
             m_data.bitstream.insert(m_data.bitstream.end(), bool(dec & (1 << i)));
     }
 
-    if (!m_data.update_used_states(bitstream_start_idx)) {
-        BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << ": dropping malformed paint data of triangle " << triangle_id;
-        m_data.bitstream.resize(bitstream_start_idx);
-        m_data.triangles_to_split.pop_back();
-    }
+    m_data.update_used_states(bitstream_start_idx);
 }
 
 bool FacetsAnnotation::equals(const FacetsAnnotation &other) const

src/libslic3r/TriangleSelector.cpp

@@ -1778,13 +1778,6 @@ TriangleSelector::TriangleSplittingData TriangleSelector::serialize() const {
     return out.data;
 }
 
-// A split code keeps the split side (one split) or the kept side (two splits) in its upper two
-// bits, where 3 is not a side. The value is ignored for a three-side split.
-static bool split_code_valid(int code)
-{
-    return (code & 0b11) == 3 || (code >> 2) != 3;
-}
-
 void TriangleSelector::deserialize(const TriangleSplittingData &data,
                                    bool                         needs_reset,
                                    EnforcerBlockerType          max_ebt,
@@ -1819,12 +1812,11 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
 
     for (auto [triangle_id, ibit] : data.triangles_to_split) {
         assert(triangle_id < int(m_triangles.size()));
-        // Set when the bitstream runs out or holds an impossible split before this triangle's tree is complete.
-        bool corrupt = false;
-        auto next_nibble = [&data, &ibit = ibit, &corrupt]() {
+        assert(ibit < int(data.bitstream.size()));
+        auto next_nibble = [&data, &ibit = ibit]() {
             int n = 0;
-            if (! data.read_nibble(ibit, n))
-                corrupt = true;
+            for (int i = 0; i < 4; ++ i)
+                n |= data.bitstream[ibit ++] << i;
             return n;
         };
         // Decode a leaf state stored behind the "11" prefix: one nibble of (state-3) for states
@@ -1843,10 +1835,6 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
             bool is_split = num_of_children != 0;
             // Only valid if not is_split.
             auto state = is_split ? EnforcerBlockerType::NONE : ((code & 0b1100) == 0b1100 ? decode_leaf_state() : EnforcerBlockerType(code >> 2));
-            if (is_split && ! split_code_valid(code))
-                corrupt = true;
-            if (corrupt)
-                break;
 
             // BBS
             if (state == to_delete_filament)
@@ -1861,7 +1849,7 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
             }
 
             // Only valid if is_split.
-            int special_side = num_of_split_sides == 3 ? 0 : code >> 2;
+            int special_side = code >> 2;
 
             // Take care of the first iteration separately, so handling of the others is simpler.
             if (parents.empty()) {
@@ -1916,55 +1904,47 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
             if (parents.empty())
                 break;
         }
-
-        if (corrupt) {
-            // Every split above allocated all of its children, so the partial tree unwinds cleanly.
-            BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << ": malformed paint data, dropping paint of triangle " << triangle_id;
-            undivide_triangle(triangle_id);
-            m_triangles[triangle_id].set_state(EnforcerBlockerType::NONE);
-        }
     }
 }
 
-bool TriangleSelector::TriangleSplittingData::update_used_states(const size_t bitstream_start_idx) {
-    int      ibit   = static_cast<int>(bitstream_start_idx);
-    uint64_t states = 0;
-    do {
-        // Walk one triangle's tree depth-first, counting the nodes still to be read; a split node adds its children.
-        for (int pending_nodes = 1; pending_nodes > 0; --pending_nodes) {
-            int code;
-            if (!this->read_nibble(ibit, code))
-                return false;
+void TriangleSelector::TriangleSplittingData::update_used_states(const size_t bitstream_start_idx) {
+    assert(bitstream_start_idx < this->bitstream.size());
+    assert(!this->bitstream.empty() && this->bitstream.size() != bitstream_start_idx);
+    assert((this->bitstream.size() - bitstream_start_idx) % 4 == 0);
 
-            if (const int num_of_split_sides = code & 0b11; num_of_split_sides != 0) {
-                if (!split_code_valid(code))
-                    return false;
-                pending_nodes += num_of_split_sides + 1;
-                continue;
-            }
+    if (this->bitstream.empty() || this->bitstream.size() == bitstream_start_idx)
+        return;
 
-            int facet_state = code >> 2;
-            if (facet_state == 0b11) {
-                // Leaf behind the "11" prefix: one nibble of (state-3), or 0b1111 + (state-18).
-                int nibble;
-                if (!this->read_nibble(ibit, nibble))
-                    return false;
-                facet_state = nibble + 3;
-                if (nibble == 0b1111) {
-                    if (!this->read_nibble(ibit, nibble))
-                        return false;
-                    facet_state = nibble + 18;
-                }
-            }
-            states |= uint64_t(1) << facet_state;
+    size_t nibble_idx = bitstream_start_idx;
+
+    auto read_next_nibble = [&data_bitstream = std::as_const(this->bitstream), &nibble_idx]() -> uint8_t {
+        assert(nibble_idx + 3 < data_bitstream.size());
+        uint8_t code = 0;
+        for (size_t bit_idx = 0; bit_idx < 4; ++bit_idx)
+            code |= data_bitstream[nibble_idx++] << bit_idx;
+        return code;
+    };
+
+    while (nibble_idx < this->bitstream.size()) {
+        const uint8_t code = read_next_nibble();
+
+        if (const bool is_split = (code & 0b11) != 0; is_split)
+            continue;
+
+        uint8_t facet_state;
+        if ((code & 0b1100) == 0b1100) {
+            // Leaf behind the "11" prefix: one nibble of (state-3), or 0b1111 + (state-18).
+            const uint8_t nibble = read_next_nibble();
+            facet_state = nibble == 0b1111 ? uint8_t(read_next_nibble() + 18) : uint8_t(nibble + 3);
+        } else {
+            facet_state = code >> 2;
         }
-    } while (static_cast<size_t>(ibit) < this->bitstream.size());
+        assert(facet_state < this->used_states.size());
+        if (facet_state >= this->used_states.size())
+            continue;
 
-    // The leaf encoding tops out at state 33, so every state fits the 64-bit mask.
-    for (size_t state_idx = 0; state_idx < std::min<size_t>(this->used_states.size(), 64); ++state_idx)
-        if (states & (uint64_t(1) << state_idx))
-            this->used_states[state_idx] = true;
-    return true;
+        this->used_states[facet_state] = true;
+    }
 }
 
 // Lightweight variant of deserialization, which only tests whether a face of test_state exists.
@@ -1976,12 +1956,11 @@ bool TriangleSelector::has_facets(const TriangleSplittingData &data, const Enfor
 
     for (const TriangleBitStreamMapping &triangle_id_and_ibit : data.triangles_to_split) {
         int ibit = triangle_id_and_ibit.bitstream_start_idx;
-        // Stop reading a triangle whose stream is truncated.
-        bool truncated = false;
-        auto next_nibble = [&data, &ibit = ibit, &truncated]() {
+        assert(ibit < int(data.bitstream.size()));
+        auto next_nibble = [&data, &ibit = ibit]() {
             int n = 0;
-            if (! data.read_nibble(ibit, n))
-                truncated = true;
+            for (int i = 0; i < 4; ++ i)
+                n |= data.bitstream[ibit ++] << i;
             return n;
         };
         // < 0 -> negative of a number of children
@@ -1999,8 +1978,6 @@ bool TriangleSelector::has_facets(const TriangleSplittingData &data, const Enfor
         };
 
         int state = num_children_or_state();
-        if (truncated)
-            continue;
         if (state < 0) {
             // Root is split.
             parents_children.clear();
@@ -2008,8 +1985,6 @@ bool TriangleSelector::has_facets(const TriangleSplittingData &data, const Enfor
             do {
                 if (-- parents_children.back() >= 0) {
                     int state = num_children_or_state();
-                    if (truncated)
-                        break;
                     if (state < 0)
                         // Child is split.
                         parents_children.emplace_back(- state);

src/libslic3r/TriangleSelector.hpp

@@ -297,20 +297,8 @@ public:
             std::fill(used_states.begin(), used_states.end(), false);
         }
 
-        // Update used states from the triangle trees stored between bitstream_start_idx and the end of the bitstream.
-        // Returns false and leaves used states untouched if a tree is truncated or malformed.
-        bool update_used_states(size_t bitstream_start_idx);
-
-        // Read the 4-bit code at bit index ibit (LSB first) and advance ibit past it.
-        // Returns false without advancing when fewer than 4 bits remain.
-        bool read_nibble(int &ibit, int &nibble) const {
-            if (ibit < 0 || static_cast<size_t>(ibit) + 4 > bitstream.size())
-                return false;
-            nibble = 0;
-            for (int i = 0; i < 4; ++i)
-                nibble |= static_cast<int>(bitstream[ibit++]) << i;
-            return true;
-        }
+        // Update used states based on the bitstream. It just iterated over the bitstream from the bitstream_start_idx till the end.
+        void update_used_states(size_t bitstream_start_idx);
 
     private:
         friend class cereal::access;

src/libslic3r/Utils.hpp

@@ -260,6 +260,9 @@ extern bool is_json_file(const std::string& path);
 // 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"); }

src/libslic3r/miniz_extension.cpp

@@ -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);

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:

src/libslic3r/utils.cpp

@@ -1103,7 +1103,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 +1115,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");

src/slic3r/GUI/GUI_App.cpp

@@ -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) {

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 "<<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.

tests/libslic3r/CMakeLists.txt

@@ -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

tests/libslic3r/test_3mf.cpp

@@ -19,7 +19,6 @@
 #include <boost/filesystem/operations.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 #include <algorithm>
-#include <functional>
 
 #include <catch2/catch_tostring.hpp>
 #include <Eigen/Core>
@@ -185,13 +184,16 @@ static bool read_cad_recipe_entry(const std::string& path, std::string& out,
     return found;
 }
 
-// Rewrites the archive at `path`, letting `edit` change the name or data of each entry; returns
-// whether `edit` reported a change for any of them. miniz cannot edit in place and
-// open_zip_writer truncates, so the entries are held across the switch.
-static bool rewrite_3mf_entries(const std::string& path, const std::function<bool(std::string& name, std::string& data)>& edit)
+// Rewrites the archive at `path` with the recipe entry back under the name it had before the
+// rename, which is what every project saved by an earlier build looks like on disk. Generated
+// rather than checked in because a whole project archive is not frozen evidence the way a bare
+// recipe blob is -- it has to be whatever today's exporter writes, with only the name aged.
+// miniz cannot rename in place and open_zip_writer truncates, so the entries are held across
+// the switch.
+static void rename_cad_recipe_entry_to_legacy(const std::string& path)
 {
     std::vector<std::pair<std::string, std::string>> entries;
-    bool changed = false;
+    bool renamed = false;
     {
         mz_zip_archive zip;
         mz_zip_zero_struct(&zip);
@@ -206,140 +208,22 @@ static bool rewrite_3mf_entries(const std::string& path, const std::function<boo
             std::string data((size_t) st.m_uncomp_size, '\0');
             if (st.m_uncomp_size > 0)
                 REQUIRE(mz_zip_reader_extract_to_mem(&zip, i, data.data(), data.size(), 0));
-            changed |= edit(name, data);
+            if (boost::algorithm::iequals(name, CAD_RECIPE_ENTRY)) {
+                name    = LEGACY_CAD_RECIPE_ENTRY;
+                renamed = true;
+            }
             entries.emplace_back(std::move(name), std::move(data));
         }
         close_zip_reader(&zip);
     }
+    // Without this the scenario would degrade silently into re-testing the new name if the
+    // exporter's constant ever moved again: every load below would still pass.
+    REQUIRE(renamed);
 
     Zipper out(path);
     for (const auto& e : entries)
         out.add_entry(e.first, e.second.data(), e.second.size());
     out.finalize();
-    return changed;
-}
-
-// Rewrites the archive at `path` with the recipe entry back under the name it had before the
-// rename, which is what every project saved by an earlier build looks like on disk. Generated
-// rather than checked in because a whole project archive is not frozen evidence the way a bare
-// recipe blob is -- it has to be whatever today's exporter writes, with only the name aged.
-static void rename_cad_recipe_entry_to_legacy(const std::string& path)
-{
-    const bool renamed = rewrite_3mf_entries(path, [](std::string& name, std::string&) {
-        if (!boost::algorithm::iequals(name, CAD_RECIPE_ENTRY))
-            return false;
-        name = LEGACY_CAD_RECIPE_ENTRY;
-        return true;
-    });
-    // Without this the scenario would degrade silently into re-testing the new name if the
-    // exporter's constant ever moved again: every load below would still pass.
-    REQUIRE(renamed);
-}
-
-// Replaces the first occurrence of `from` in any entry whose name ends with `suffix`.
-static bool replace_in_3mf_entry(const std::string& path, const std::string& suffix, const std::string& from, const std::string& to)
-{
-    bool replaced = false;
-    rewrite_3mf_entries(path, [&](std::string& name, std::string& data) {
-        if (replaced || !boost::algorithm::ends_with(name, suffix))
-            return false;
-        if (const size_t pos = data.find(from); pos != std::string::npos) {
-            data.replace(pos, from.size(), to);
-            replaced = true;
-        }
-        return replaced;
-    });
-    return replaced;
-}
-
-// Stores a one-plate project holding a cube whose first two facets are painted Extruder2 and
-// Extruder3, which the exporter writes as paint_color="8" and paint_color="0C".
-static void store_painted_cube(const std::string& path)
-{
-    Model        model;
-    ModelObject* object = model.add_object();
-    ModelVolume* volume = object->add_volume(make_cube(10., 10., 10.));
-    object->add_instance();
-    {
-        TriangleSelector selector(volume->mesh());
-        selector.set_facet(0, EnforcerBlockerType::Extruder2);
-        selector.set_facet(1, EnforcerBlockerType::Extruder3);
-        REQUIRE(volume->mmu_segmentation_facets.set(selector));
-    }
-    ScopedTemporaryDir backup_dir("orca_paint_src");
-    model.set_backup_path(backup_dir.string());
-
-    DynamicPrintConfig cfg;
-    PlateData          plate;
-    plate.plate_index = 0;
-    StoreParams sp;
-    sp.path     = path.c_str();
-    sp.model    = &model;
-    sp.config   = &cfg;
-    sp.strategy = SaveStrategy::Zip64 | SaveStrategy::Silence;
-    sp.plate_data_list.push_back(&plate);
-    REQUIRE(store_bbs_3mf(sp));
-}
-
-// Loads `path` through the BBS importer into `model`, releasing the plates it returns. The
-// importer stages metadata through `backup_dir`, which has to outlive the model.
-static bool load_project(const std::string& path, Model& model, const ScopedTemporaryDir& backup_dir)
-{
-    model.set_backup_path(backup_dir.string());
-    DynamicPrintConfig        config;
-    ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Enable };
-    PlateDataPtrs             plates;
-    std::vector<Preset*>      project_presets;
-    bool   is_bbl_3mf = false, is_orca_3mf = false;
-    Semver file_version;
-    const bool loaded = load_bbs_3mf(path.c_str(), &config, &ctxt, &model, &plates, &project_presets, &is_bbl_3mf,
-                                     &is_orca_3mf, &file_version, nullptr, LoadStrategy::LoadModel | LoadStrategy::LoadConfig);
-    release_PlateData_list(plates);
-    return loaded;
-}
-
-TEST_CASE("A project with a plate id below 1 fails to load", "[3mf][Regression]")
-{
-    const int plate_id = GENERATE(0, -1);
-    INFO("plater_id " << plate_id);
-
-    ScopedTemporaryFile temp(".3mf");
-    store_painted_cube(temp.string());
-    {
-        ScopedTemporaryDir backup_dir("orca_plate_dst");
-        Model              model;
-        REQUIRE(load_project(temp.string(), model, backup_dir));
-    }
-
-    REQUIRE(replace_in_3mf_entry(temp.string(), "model_settings.config", "key=\"plater_id\" value=\"1\"",
-                                 "key=\"plater_id\" value=\"" + std::to_string(plate_id) + "\""));
-    ScopedTemporaryDir backup_dir("orca_plate_dst");
-    Model              model;
-    bool               loaded = true;
-    REQUIRE_NOTHROW(loaded = load_project(temp.string(), model, backup_dir));
-    REQUIRE_FALSE(loaded);
-}
-
-TEST_CASE("A project with malformed paint data loads without the damaged facet", "[3mf][Regression]")
-{
-    ScopedTemporaryFile temp(".3mf");
-    store_painted_cube(temp.string());
-    // Split codes with no children behind them: the stream runs out mid-tree.
-    REQUIRE(replace_in_3mf_entry(temp.string(), ".model", "paint_color=\"8\"", "paint_color=\"FFFFFFFFFFFFFFFF3\""));
-
-    ScopedTemporaryDir backup_dir("orca_paint_dst");
-    Model              model;
-    REQUIRE(load_project(temp.string(), model, backup_dir));
-    REQUIRE(model.objects.size() == 1);
-    const ModelVolume& volume = *model.objects.front()->volumes.front();
-    const auto&        data   = volume.mmu_segmentation_facets.get_data();
-    REQUIRE_FALSE(data.used_states[size_t(EnforcerBlockerType::Extruder2)]);
-    REQUIRE(data.used_states[size_t(EnforcerBlockerType::Extruder3)]);
-
-    TriangleSelector selector(volume.mesh());
-    REQUIRE_NOTHROW(selector.deserialize(data));
-    REQUIRE(selector.num_facets(EnforcerBlockerType::Extruder2) == 0);
-    REQUIRE(selector.num_facets(EnforcerBlockerType::Extruder3) == 1);
 }
 
 // The recipe lives only in the BBS-native backend, because that is the only one that runs:

tests/libslic3r/test_miniz_extension.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

tests/libslic3r/test_triangle_selector.cpp

@@ -3,8 +3,6 @@
 #include "libslic3r/TriangleSelector.hpp"
 #include "libslic3r/TriangleMesh.hpp"
 
-#include <algorithm>
-
 using namespace Slic3r;
 
 // A sphere gives well over ExtruderMax original facets, so every extruder state can be assigned
@@ -125,77 +123,3 @@ TEST_CASE("Extruder states match the CONST_FILAMENTS hex encoding", "[TriangleSe
     INFO("Hex " << c.hex << " -> extruder " << c.state);
     REQUIRE(TriangleSelector::has_facets(data, EnforcerBlockerType(c.state)));
 }
-
-// Pack 4-bit codes into a bitstream, least significant bit first, in the order the decoder reads them.
-static std::vector<bool> pack_nibbles(const std::vector<int> &nibbles)
-{
-    std::vector<bool> bitstream;
-    for (const int nibble : nibbles)
-        for (int bit = 0; bit < 4; ++bit)
-            bitstream.push_back((nibble >> bit) & 1);
-    return bitstream;
-}
-
-TEST_CASE("A valid paint stream with nested splits round-trips bit for bit", "[TriangleSelector]")
-{
-    const TriangleMesh mesh = test_mesh();
-
-    TriangleSelector::TriangleSplittingData data;
-    data.triangles_to_split.emplace_back(0, 0);
-    // A three-side split whose children, in stream order, are: a one-side split (side 2) into two
-    // leaves, a two-side split (side 1) into leaves of states 20, 0 and 8, then two plain leaves.
-    const std::vector<int> triangle_0 = {0b0011,
-                                         0b1001, 0b1000, 0b0100,
-                                         0b0110, 0b1100, 0b1111, 20 - 18, 0b0000, 0b1100, 8 - 3,
-                                         0b1000,
-                                         0b0100};
-    data.bitstream = pack_nibbles(triangle_0);
-    data.triangles_to_split.emplace_back(5, int(data.bitstream.size()));
-    const std::vector<bool> triangle_5 = pack_nibbles({0b1100, 3 - 3});
-    data.bitstream.insert(data.bitstream.end(), triangle_5.begin(), triangle_5.end());
-    data.reset_used_states();
-    REQUIRE(data.update_used_states(0));
-
-    TriangleSelector restored(mesh);
-    restored.deserialize(data);
-
-    REQUIRE(restored.num_facets(EnforcerBlockerType::Extruder20) == 1);
-    REQUIRE(restored.num_facets(EnforcerBlockerType::Extruder3) == 1);
-    REQUIRE(restored.serialize() == data);
-}
-
-TEST_CASE("A truncated or malformed paint stream drops only the damaged triangle", "[TriangleSelector][Regression]")
-{
-    struct Case { const char *name; std::vector<int> nibbles; };
-    const auto c = GENERATE(values<Case>({
-        {"three-side split missing two children", {0b0011, 0b1000, 0b1000}},
-        {"leaf missing its state nibble",         {0b1100}},
-        {"leaf missing its second state nibble",  {0b1100, 0b1111}},
-        {"splits nested past the end",            {0b0011, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF,
-                                                    0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF}},
-        {"one-side split of the nonexistent side 3", {0b1101, 0b1000, 0b1000}},
-    }));
-    INFO(c.name);
-
-    const TriangleMesh mesh = test_mesh();
-    TriangleSelector   intact(mesh);
-    intact.set_facet(0, EnforcerBlockerType::Extruder2);
-
-    // Triangle 0 stays intact, triangle 1 carries the damaged stream.
-    TriangleSelector::TriangleSplittingData data = intact.serialize();
-    data.triangles_to_split.emplace_back(1, int(data.bitstream.size()));
-    const std::vector<bool> damaged = pack_nibbles(c.nibbles);
-    data.bitstream.insert(data.bitstream.end(), damaged.begin(), damaged.end());
-
-    TriangleSelector restored(mesh);
-    REQUIRE_NOTHROW(restored.deserialize(data));
-    // Triangle 1 unwinds completely, so the selector holds exactly the intact paint.
-    REQUIRE(restored.serialize() == intact.serialize());
-
-    REQUIRE_NOTHROW(TriangleSelector::has_facets(data, EnforcerBlockerType::Extruder3));
-
-    TriangleSelector::TriangleSplittingData recomputed = data;
-    recomputed.reset_used_states();
-    REQUIRE_FALSE(recomputed.update_used_states(0));
-    REQUIRE(std::none_of(recomputed.used_states.begin(), recomputed.used_states.end(), [](bool used) { return used; }));
-}

tests/libslic3r/test_utils.cpp

@@ -152,3 +152,71 @@ TEST_CASE("resolve_cli_input_path leaves inputs that must not be completed uncha
         REQUIRE(resolve_cli_input_path("").empty());
     }
 }
+
+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("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()));
+    }
+}
+
+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("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")));
+    CAPTURE(link, target);
+    CHECK_FALSE(is_symlink_target_within_root(link, target, tmp.path()));
+}
+
+#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");
+
+    CHECK_FALSE(is_symlink_target_within_root("link", "out/lib.so", root));
+    CHECK(is_symlink_target_within_root("link", "in/lib.so", root));
+}
+#endif