From 306b09b4f55316d03437dad2e1ee94812745a316 Mon Sep 17 00:00:00 2001 From: Momin Al-Ghosien Date: Sun, 28 Apr 2024 18:03:34 -0700 Subject: [PATCH] Fix use after free bug in LinesBucketQueue::emplace_back_bucket I found a use after free bug in LinesBucketQueue::emplace_back_bucket. This was found by enabling address sanitizer. The LinesBucketQueue class has two related members: std::vector line_buckets; std::priority_queue, LinesBucketPtrComp> line_bucket_ptr_queue; line_bucket_ptr_queue holds pointers into line_buckets. However, since items are inserted into line_buckets one at a time, existing pointers that were stored inside line_bucket_ptr_queue become invalid. Specifically: void LinesBucketQueue::emplace_back_bucket(ExtrusionLayers &&els, const void *objPtr, Point offset) { auto oldSize = line_buckets.capacity(); line_buckets.emplace_back(std::move(els), objPtr, offset); <--- Causes a reallocation, making previous pointers invalid line_bucket_ptr_queue.push(&line_buckets.back()); <-- priority queue compares against old, now invalid pointers ... The proposed fix is to calculate the required number of entries in ConflictChecker::find_inter_of_lines_in_diff_objs, and then calling line_buckets.reserve(count). This ensures that sufficient buffer is allocated up front and the pointers are stable as items are added. --- src/libslic3r/GCode/ConflictChecker.cpp | 13 +++++++++++++ src/libslic3r/GCode/ConflictChecker.hpp | 1 + 2 files changed, 14 insertions(+) diff --git a/src/libslic3r/GCode/ConflictChecker.cpp b/src/libslic3r/GCode/ConflictChecker.cpp index 996615409..ad7493d66 100644 --- a/src/libslic3r/GCode/ConflictChecker.cpp +++ b/src/libslic3r/GCode/ConflictChecker.cpp @@ -102,6 +102,11 @@ void LinesBucketQueue::emplace_back_bucket(ExtrusionLayers &&els, const void *ob } } +void LinesBucketQueue::reserve(size_t count) +{ + line_buckets.reserve(count); +} + // remove lowest and get the current bottom z float LinesBucketQueue::getCurrBottomZ() { @@ -218,6 +223,14 @@ ConflictResultOpt ConflictChecker::find_inter_of_lines_in_diff_objs(PrintObjectP { if (objs.size() <= 1 && !wtdptr) { return {}; } LinesBucketQueue conflictQueue; + + // Calculate the number of required entries in the conflict queue + // One slot is used for the FakeWipeTower and 2 slots for each object + size_t requiredCount = 2*objs.size() + (wtdptr.has_value() ? 1 : 0); + // Reserve the required count to guarantee that pointers inside LinesBucketQueue::line_buckets + // are stable while we append the entries + conflictQueue.reserve(requiredCount); + if (wtdptr.has_value()) { // wipe tower at 0 by default auto wtpaths = wtdptr.value()->getFakeExtrusionPathsFromWipeTower(); ExtrusionLayers wtels; diff --git a/src/libslic3r/GCode/ConflictChecker.hpp b/src/libslic3r/GCode/ConflictChecker.hpp index eac10aac4..4937a209f 100644 --- a/src/libslic3r/GCode/ConflictChecker.hpp +++ b/src/libslic3r/GCode/ConflictChecker.hpp @@ -114,6 +114,7 @@ public: public: void emplace_back_bucket(ExtrusionLayers &&els, const void *objPtr, Point offset); + void reserve(size_t count); bool valid() const { return line_bucket_ptr_queue.empty() == false; } float getCurrBottomZ(); LineWithIDs getCurLines() const;