From 80baf70b01a57b3445cfe5f731ac3a9d0e6885fc Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Mon, 28 Sep 2026 06:00:18 +0800 Subject: [PATCH] Drop Malformed 3MF Paint Data Instead of Reading Past the Bitstream Painted facets are decoded from a bitstream a nibble at a time with no bound check, so a truncated or corrupt paint string in a 3MF (for example split codes with no children behind them) read past the end and crashed on load and slice. A one- or two-side split naming side 3 also indexed past the triangle's vertices. Every nibble read now goes through a bounds-checked reader. Loading validates each triangle's tree and drops a malformed one with a warning, so the stored data, used extruder states and later decoding all agree. deserialize() also unwinds and clears any triangle whose tree is incomplete or malformed, and has_facets() stops at a truncated triangle. Valid streams decode unchanged. --- src/libslic3r/Model.cpp | 6 +- src/libslic3r/TriangleSelector.cpp | 111 +++++++++++++-------- src/libslic3r/TriangleSelector.hpp | 16 ++- tests/libslic3r/test_3mf.cpp | 22 ++++ tests/libslic3r/test_triangle_selector.cpp | 76 ++++++++++++++ 5 files changed, 185 insertions(+), 46 deletions(-) diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp index 209ccdafa2..63dbf7390f 100644 --- a/src/libslic3r/Model.cpp +++ b/src/libslic3r/Model.cpp @@ -3702,7 +3702,11 @@ void FacetsAnnotation::set_triangle_from_string(int triangle_id, const std::stri m_data.bitstream.insert(m_data.bitstream.end(), bool(dec & (1 << i))); } - m_data.update_used_states(bitstream_start_idx); + 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(); + } } bool FacetsAnnotation::equals(const FacetsAnnotation &other) const diff --git a/src/libslic3r/TriangleSelector.cpp b/src/libslic3r/TriangleSelector.cpp index b47004fca5..3dfea4c0c7 100644 --- a/src/libslic3r/TriangleSelector.cpp +++ b/src/libslic3r/TriangleSelector.cpp @@ -1778,6 +1778,13 @@ 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, @@ -1812,11 +1819,12 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data, for (auto [triangle_id, ibit] : data.triangles_to_split) { assert(triangle_id < int(m_triangles.size())); - assert(ibit < int(data.bitstream.size())); - auto next_nibble = [&data, &ibit = ibit]() { + // 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]() { int n = 0; - for (int i = 0; i < 4; ++ i) - n |= data.bitstream[ibit ++] << i; + if (! data.read_nibble(ibit, n)) + corrupt = true; return n; }; // Decode a leaf state stored behind the "11" prefix: one nibble of (state-3) for states @@ -1835,6 +1843,10 @@ 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) @@ -1849,7 +1861,7 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data, } // Only valid if is_split. - int special_side = code >> 2; + int special_side = num_of_split_sides == 3 ? 0 : code >> 2; // Take care of the first iteration separately, so handling of the others is simpler. if (parents.empty()) { @@ -1904,47 +1916,55 @@ 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); + } } } -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); +bool TriangleSelector::TriangleSplittingData::update_used_states(const size_t bitstream_start_idx) { + int ibit = static_cast(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; - if (this->bitstream.empty() || this->bitstream.size() == bitstream_start_idx) - return; + 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; + } - 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; + 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; } - assert(facet_state < this->used_states.size()); - if (facet_state >= this->used_states.size()) - continue; + } while (static_cast(ibit) < this->bitstream.size()); - this->used_states[facet_state] = true; - } + // 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(this->used_states.size(), 64); ++state_idx) + if (states & (uint64_t(1) << state_idx)) + this->used_states[state_idx] = true; + return true; } // Lightweight variant of deserialization, which only tests whether a face of test_state exists. @@ -1956,11 +1976,12 @@ 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; - assert(ibit < int(data.bitstream.size())); - auto next_nibble = [&data, &ibit = ibit]() { + // Stop reading a triangle whose stream is truncated. + bool truncated = false; + auto next_nibble = [&data, &ibit = ibit, &truncated]() { int n = 0; - for (int i = 0; i < 4; ++ i) - n |= data.bitstream[ibit ++] << i; + if (! data.read_nibble(ibit, n)) + truncated = true; return n; }; // < 0 -> negative of a number of children @@ -1978,6 +1999,8 @@ 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(); @@ -1985,6 +2008,8 @@ 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); diff --git a/src/libslic3r/TriangleSelector.hpp b/src/libslic3r/TriangleSelector.hpp index 594f710e45..2e6627039c 100644 --- a/src/libslic3r/TriangleSelector.hpp +++ b/src/libslic3r/TriangleSelector.hpp @@ -297,8 +297,20 @@ public: std::fill(used_states.begin(), used_states.end(), false); } - // 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); + // 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(ibit) + 4 > bitstream.size()) + return false; + nibble = 0; + for (int i = 0; i < 4; ++i) + nibble |= static_cast(bitstream[ibit++]) << i; + return true; + } private: friend class cereal::access; diff --git a/tests/libslic3r/test_3mf.cpp b/tests/libslic3r/test_3mf.cpp index efe5639b16..ce3285c7ad 100644 --- a/tests/libslic3r/test_3mf.cpp +++ b/tests/libslic3r/test_3mf.cpp @@ -320,6 +320,28 @@ TEST_CASE("A project with a plate id below 1 fails to load", "[3mf][Regression]" 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: // store_bbs_3mf is the sole exporter the app calls, and 3mf.cpp's load_3mf is reached only for // files fingerprinted as PrusaSlicer's, which never carry a recipe. This locks in both halves: diff --git a/tests/libslic3r/test_triangle_selector.cpp b/tests/libslic3r/test_triangle_selector.cpp index fd2ab9efa8..c207ee66ad 100644 --- a/tests/libslic3r/test_triangle_selector.cpp +++ b/tests/libslic3r/test_triangle_selector.cpp @@ -3,6 +3,8 @@ #include "libslic3r/TriangleSelector.hpp" #include "libslic3r/TriangleMesh.hpp" +#include + using namespace Slic3r; // A sphere gives well over ExtruderMax original facets, so every extruder state can be assigned @@ -123,3 +125,77 @@ 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 pack_nibbles(const std::vector &nibbles) +{ + std::vector 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 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 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 nibbles; }; + const auto c = GENERATE(values({ + {"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 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; })); +}