Fix TopologyPreservingSimplifier to remove ring endpoints safely (#1110)

libgeos · Jun 14, 2024 · 50ae3bd · 50ae3bd
1 parent 37f7600
commit 50ae3bd
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 4 deletions.
diff --git a/include/geos/simplify/TaggedLineString.h b/include/geos/simplify/TaggedLineString.h
@@ -105,7 +105,7 @@ class GEOS_DLL TaggedLineString {
 
     void addToResult(std::unique_ptr<TaggedLineSegment> seg);
 
-    void removeRingEndpoint();
+    const TaggedLineSegment* removeRingEndpoint();
 
     std::unique_ptr<geom::Geometry> asLineString() const;
 

diff --git a/src/simplify/TaggedLineString.cpp b/src/simplify/TaggedLineString.cpp
@@ -281,15 +281,16 @@ TaggedLineString::addToResult(std::unique_ptr<TaggedLineSegment> seg)
 #endif
 }
 
-void
+const TaggedLineSegment*
 TaggedLineString::removeRingEndpoint()
 {
     auto* firstSeg = resultSegs.front();
     auto* lastSeg = resultSegs.back();
 
     firstSeg->p0 = lastSeg->p0;
-    delete lastSeg;
     resultSegs.pop_back();
+    delete lastSeg;
+    return firstSeg;
 }
 
 } // namespace geos::simplify

diff --git a/src/simplify/TaggedLineStringSimplifier.cpp b/src/simplify/TaggedLineStringSimplifier.cpp
@@ -187,7 +187,15 @@ TaggedLineStringSimplifier::simplifyRingEndpoint(double distanceTolerance)
         if (simpSeg.distance(endPt) <= distanceTolerance &&
             isTopologyValid(line, firstSeg, lastSeg, simpSeg))
         {
-            line->removeRingEndpoint();
+            //-- don't know if segments are original or new, so remove from all indexes
+            inputIndex->remove(firstSeg);
+            inputIndex->remove(lastSeg);
+            outputIndex->remove(firstSeg);
+            outputIndex->remove(lastSeg);
+
+            const TaggedLineSegment* flatSeg = line->removeRingEndpoint();
+            //-- removed endpoint alters an existing result edge
+            outputIndex->add(flatSeg);
         }
     }
 }

diff --git a/tests/unit/simplify/TopologyPreservingSimplifierTest.cpp b/tests/unit/simplify/TopologyPreservingSimplifierTest.cpp
@@ -419,6 +419,16 @@ void object::test<32>()
     checkTPSNoChange("LINESTRING (1 0, 2 0, 2 2, 0 2, 0 0, 1 0)", 0);
 }
 
+// Test for no use-after-free triggered by ring endpoint removal handling 
+// See https://github.com/libgeos/geos/issues/1107, fix https://github.com/libgeos/geos/pull/1110
+template<>
+template<>
+void object::test<33>()
+{
+    checkTPS("POLYGON ((-222601.33094265286 6299915.50260568, -222599.13611514607 6299917.747821213, -222599.09754554977 6299925.149899498, -222599.07870256738 6299925.234615005, -222595.52372420163 6299932.934861557, -222510 6300300, -221720.85158014414 6300132.680680807, -222448.77936063593 6299647.669870703, -222618.41756525903 6299886.966175825, -222618.40178141624 6299887.020684309, -222616.09648739762 6299892.144482262, -222601.33094265286 6299915.50260568), (-222456.68914400978 6299947.489843342, -222455.07603815367 6299939.772017001, -222455.07542453965 6299939.691595431, -222456.57057590774 6299931.950053182, -222462.75368034307 6299916.87367865, -222465.9575074078 6299911.582542387, -222465.99782740293 6299911.534645676, -222483.5296791599 6299864.079888968, -222484.09789251382 6299852.105440594, -222485.11401620077 6299846.692173677, -222485.13170293145 6299846.639453617, -222487.58585740798 6299841.7086228, -222490.56062790897 6299837.359974515, -222415.1394852571 6299926.6972160805, -222421.19226152284 6299963.389132584, -222467.0202338936 6299970.185860572, -222465.71145777934 6299968.200784476, -222465.69955208438 6299968.180154371, -222464.63560265294 6299966.053788407, -222456.68914400978 6299947.489843342))",
+        20,
+        "POLYGON ((-222618.41756525903 6299886.966175825, -222510 6300300, -221720.85158014414 6300132.680680807, -222448.77936063593 6299647.669870703, -222618.41756525903 6299886.966175825), (-222467.0202338936 6299970.185860572, -222456.57057590774 6299931.950053182, -222490.56062790897 6299837.359974515, -222415.1394852571 6299926.6972160805, -222421.19226152284 6299963.389132584, -222467.0202338936 6299970.185860572))");
+}
 
 
 } // namespace tut