From d35ea27ea56f0c164ca5763ab0a6074673ebc58b Mon Sep 17 00:00:00 2001 From: HanifKoh <76276251+HanifKoh@users.noreply.github.com> Date: Tue, 29 Sep 2026 02:32:50 +0800 Subject: [PATCH] Validate Zip Entry Sizes Before Parsing 3MF XML (#15958) The 3MF importers read XML entries into a single expat buffer whose size is an int, while the archive extraction used the entry's 64-bit declared size. The two could disagree for entries declaring more than INT_MAX bytes. Reject such entries before allocating, and use one size for the buffer, the extraction and the parse. This applies to the BBS importer, the PrusaSlicer importer and the PrusaSlicer fingerprint probe. The load now fails with an error instead. --- src/libslic3r/Format/3mf.cpp | 22 ++++++--- src/libslic3r/Format/bbs_3mf.cpp | 13 ++++-- tests/libslic3r/test_3mf.cpp | 77 ++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 9 deletions(-) diff --git a/src/libslic3r/Format/3mf.cpp b/src/libslic3r/Format/3mf.cpp index 21805d258c..738f1f5f6f 100644 --- a/src/libslic3r/Format/3mf.cpp +++ b/src/libslic3r/Format/3mf.cpp @@ -311,14 +311,17 @@ bool PrusaFileParser::check_3mf_from_prusa(const std::string filename) mz_zip_archive_file_stat stat; if (!mz_zip_reader_file_stat(&archive, model_file_index, &stat)) goto EXIT; + // expat sizes its buffer with an int, so a larger entry cannot be parsed in one piece. + if (stat.m_uncomp_size > static_cast(std::numeric_limits::max())) goto EXIT; - void *parser_buffer = XML_GetBuffer(m_parser, (int) stat.m_uncomp_size); + const int xml_size = static_cast(stat.m_uncomp_size); + void *parser_buffer = XML_GetBuffer(m_parser, xml_size); if (parser_buffer == nullptr) goto EXIT; - mz_bool res = mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, parser_buffer, (size_t) stat.m_uncomp_size, 0); + mz_bool res = mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, parser_buffer, static_cast(xml_size), 0); if (res == 0) goto EXIT; - XML_ParseBuffer(m_parser, (int) stat.m_uncomp_size, 1); + XML_ParseBuffer(m_parser, xml_size, 1); } } @@ -1357,19 +1360,26 @@ ModelVolumeType type_from_string(const std::string &s) XML_SetUserData(m_xml_parser, (void*)this); XML_SetElementHandler(m_xml_parser, _3MF_Importer::_handle_start_config_xml_element, _3MF_Importer::_handle_end_config_xml_element); - void* parser_buffer = XML_GetBuffer(m_xml_parser, (int)stat.m_uncomp_size); + // expat sizes its buffer with an int, so a larger entry cannot be parsed in one piece. + if (stat.m_uncomp_size > static_cast(std::numeric_limits::max())) { + add_error("Found invalid size"); + return false; + } + const int xml_size = static_cast(stat.m_uncomp_size); + + void* parser_buffer = XML_GetBuffer(m_xml_parser, xml_size); if (parser_buffer == nullptr) { add_error("Unable to create buffer"); return false; } - mz_bool res = mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, parser_buffer, (size_t)stat.m_uncomp_size, 0); + mz_bool res = mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, parser_buffer, static_cast(xml_size), 0); if (res == 0) { add_error("Error while reading config data to buffer"); return false; } - if (!XML_ParseBuffer(m_xml_parser, (int)stat.m_uncomp_size, 1)) { + if (!XML_ParseBuffer(m_xml_parser, xml_size, 1)) { char error_buf[1024]; ::sprintf(error_buf, "Error (%s) while parsing xml file at line %d", XML_ErrorString(XML_GetErrorCode(m_xml_parser)), (int)XML_GetCurrentLineNumber(m_xml_parser)); add_error(error_buf); diff --git a/src/libslic3r/Format/bbs_3mf.cpp b/src/libslic3r/Format/bbs_3mf.cpp index e3d673dd20..160c169d69 100644 --- a/src/libslic3r/Format/bbs_3mf.cpp +++ b/src/libslic3r/Format/bbs_3mf.cpp @@ -2512,19 +2512,26 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) XML_SetEntityDeclHandler(m_xml_parser, nullptr); XML_SetExternalEntityRefHandler(m_xml_parser, nullptr); - void* parser_buffer = XML_GetBuffer(m_xml_parser, (int)stat.m_uncomp_size); + // expat sizes its buffer with an int, so a larger entry cannot be parsed in one piece. + if (stat.m_uncomp_size > static_cast(std::numeric_limits::max())) { + add_error("Found invalid size"); + return false; + } + const int xml_size = static_cast(stat.m_uncomp_size); + + void* parser_buffer = XML_GetBuffer(m_xml_parser, xml_size); if (parser_buffer == nullptr) { add_error("Unable to create buffer"); return false; } - mz_bool res = mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, parser_buffer, (size_t)stat.m_uncomp_size, 0); + mz_bool res = mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, parser_buffer, static_cast(xml_size), 0); if (res == 0) { add_error("Error while reading config data to buffer"); return false; } - if (!XML_ParseBuffer(m_xml_parser, (int)stat.m_uncomp_size, 1)) { + if (!XML_ParseBuffer(m_xml_parser, xml_size, 1)) { char error_buf[1024]; ::snprintf(error_buf, 1024, "Error (%s) while parsing xml file at line %d", XML_ErrorString(XML_GetErrorCode(m_xml_parser)), (int)XML_GetCurrentLineNumber(m_xml_parser)); add_error(error_buf); diff --git a/tests/libslic3r/test_3mf.cpp b/tests/libslic3r/test_3mf.cpp index ce3285c7ad..846caf5432 100644 --- a/tests/libslic3r/test_3mf.cpp +++ b/tests/libslic3r/test_3mf.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -1577,3 +1578,79 @@ SCENARIO("bbs_3mf_is_published detects only genuinely published 3MFs", "[3mf]") } } +// Writes a single-entry zip whose central directory carries a zip64 record declaring an +// uncompressed size beyond what the 32-bit expat buffer API can take, while the deflated +// payload inflates to only ~64 KiB. Built by hand because miniz never writes a size that +// disagrees with the data. +static void write_zip_with_oversized_entry(const std::string& path, const std::string& entry) +{ + const std::string xml = ""; + size_t comp_len = 0; + void* comp = tdefl_compress_mem_to_heap(xml.data(), xml.size(), &comp_len, TDEFL_DEFAULT_MAX_PROBES); + REQUIRE(comp != nullptr); + const std::string deflated(static_cast(comp), comp_len); + mz_free(comp); + const uint32_t crc = static_cast(mz_crc32(MZ_CRC32_INIT, reinterpret_cast(xml.data()), xml.size())); + const uint64_t claimed_size = (uint64_t(1) << 32) + 16; + + std::string out; + auto put = [&out](uint64_t v, int bytes) { + for (int i = 0; i < bytes; ++i) + out.push_back(static_cast((v >> (8 * i)) & 0xFF)); + }; + // local file header, with the true sizes + put(0x04034b50, 4); put(45, 2); put(0, 2); put(8, 2); put(0, 2); put(0, 2); + put(crc, 4); put(deflated.size(), 4); put(xml.size(), 4); put(entry.size(), 2); put(0, 2); + out += entry + deflated; + // central directory header, sizes deferred to the zip64 extra field + const size_t cd_offset = out.size(); + put(0x02014b50, 4); put(45, 2); put(45, 2); put(0, 2); put(8, 2); put(0, 2); put(0, 2); + put(crc, 4); put(0xFFFFFFFF, 4); put(0xFFFFFFFF, 4); put(entry.size(), 2); put(20, 2); + put(0, 2); put(0, 2); put(0, 2); put(0, 4); put(0, 4); + out += entry; + put(0x0001, 2); put(16, 2); put(claimed_size, 8); put(deflated.size(), 8); + const size_t cd_size = out.size() - cd_offset; + // end of central directory + put(0x06054b50, 4); put(0, 2); put(0, 2); put(1, 2); put(1, 2); + put(cd_size, 4); put(cd_offset, 4); put(0, 2); + + boost::nowide::ofstream f(path, std::ios::binary); + REQUIRE(f.good()); + f.write(out.data(), static_cast(out.size())); + REQUIRE(f.good()); +} + +TEST_CASE("3MF XML entries declaring more than an int can hold fail to load", "[3mf]") { + ScopedTemporaryFile temp(".3mf"); + const std::string path = temp.string(); + + SECTION("BBS importer") { + write_zip_with_oversized_entry(path, "_rels/.rels"); + Model model; + DynamicPrintConfig config; + ConfigSubstitutionContext ctxt{ForwardCompatibilitySubstitutionRule::Enable}; + PlateDataPtrs plates; + std::vector project_presets; + bool is_bbl_3mf = false, is_orca_3mf = false; + Semver file_version; + bool loaded = true; + REQUIRE_NOTHROW(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)); + CHECK_FALSE(loaded); + release_PlateData_list(plates); + } + SECTION("PrusaSlicer importer") { + write_zip_with_oversized_entry(path, "Metadata/Slic3r_PE_model.config"); + Model model; + DynamicPrintConfig config; + ConfigSubstitutionContext ctxt{ForwardCompatibilitySubstitutionRule::Disable}; + bool loaded = true; + REQUIRE_NOTHROW(loaded = load_3mf(path.c_str(), config, ctxt, &model, false)); + CHECK_FALSE(loaded); + } + SECTION("PrusaSlicer fingerprint probe") { + write_zip_with_oversized_entry(path, "3D/3dmodel.model"); + PrusaFileParser parser; + CHECK_FALSE(parser.check_3mf_from_prusa(path)); + } +}