Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TopologyPreservingSimplifier to remove ring endpoints safely #1110

Merged
merged 5 commits into from
Jun 14, 2024
+23 −4
Merged

Fix TopologyPreservingSimplifier to remove ring endpoints safely #1110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
30ad0fe
Fix TPSimplifier to remove endpoints safely
dr-jts Jun 13, 2024
93544d2
Add unit test for use-after-free error
dr-jts Jun 13, 2024
8ad5c9d
Revert fix to test CI ASAN detection
dr-jts Jun 13, 2024
c7a61f0
Reenable fixed code
dr-jts Jun 14, 2024
ba8f079
Make removeRingEndpoint return segment const
dr-jts Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/geos/simplify/TaggedLineString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
5 changes: 3 additions & 2 deletions src/simplify/TaggedLineString.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion src/simplify/TaggedLineStringSimplifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/simplify/TopologyPreservingSimplifierTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading