ST_SimplifyPreserveTopology crashes with infinity coordinates

[robe]

Same issue as #5318, #5319, #5315

SELECT ST_SimplifyPreserveTopology('0106000020E61000000100000001030000000100000005000000000000000000F07F000000000000F07F000000000000F07F000000000000F07F000000000000F07F000000000000F07F000000000000F07F000000000000F07F000000000000F07F000000000000F07F'::geometry, 20);

[robe]

backtrace for this

   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos.dll
#52141 0x0000000064c9e714 in geos::index::quadtree::Root::insert(geos::geom::Envelope const*, void*) ()
   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos.dll
#52142 0x0000000064c9e4d4 in geos::index::quadtree::Quadtree::insert(geos::geom::Envelope const*, void*) ()
   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos.dll
#52143 0x0000000064d59cee in geos::simplify::LineSegmentIndex::add(geos::simplify::TaggedLineString const&) ()
   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos.dll
#52144 0x0000000064d5ea5e in geos::simplify::TopologyPreservingSimplifier::getResultGeometry() ()
   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos.dll
#52145 0x0000000064d5ec7e in geos::simplify::TopologyPreservingSimplifier::simplify(geos::geom::Geometry const*, double) ()
   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos.dll
#52146 0x000000006a910577 in GEOSTopologyPreserveSimplify_r ()
   from C:\ming64gcc81\projects\geos\rel-3.11w64gcc81\bin\libgeos_c.dll
#52147 0x0000000063ed5c65 in topologypreservesimplify (fcinfo=0x7de8e68)
    at lwgeom_geos.c:900
#52148 0x000000000061e0b4 in ExecInterpExpr (state=0x7de8d78,
    econtext=0x7de8fc8, isnull=<optimized out>) at execExprInterp.c:1262
#52149 0x00000000006fd9c1 in ExecEvalExprSwitchContext (isNull=0x4d1eb6c,
    econtext=<optimized out>, state=0x7de8d78)
    at ../../../../src/include/executor/executor.h:341
#52150 evaluate_expr (expr=<optimized out>,
    result_type=result_type@entry=16390,
    result_typmod=result_typmod@entry=-1,
    result_collation=result_collation@entry=0) at clauses.c:4823
#52151 0x00000000006ff398 in evaluate_function (context=0x4d1f010,
    func_tuple=0x7d09490, funcvariadic=false, args=0x7dfa9c0, input_collid=0,
    result_collid=0, result_typmod=-1, result_type=16390, funcid=16806)
    at clauses.c:4325
#52152 simplify_function (funcid=16806, result_type=16390, result_typmod=-1,
    result_collid=result_collid@entry=0, input_collid=input_collid@entry=0,
    args_p=args_p@entry=0x4d1ed50, funcvariadic=funcvariadic@entry=false,
    process_args=process_args@entry=true,
    allow_non_const=allow_non_const@entry=true,
    context=context@entry=0x4d1f010) at clauses.c:3908
#52153 0x00000000006fdf72 in eval_const_expressions_mutator (node=0x7dfa610,
    context=0x4d1f010) at clauses.c:2427
#52154 0x0000000000698b36 in expression_tree_mutator (
    node=node@entry=0x7dfa5b8,
    mutator=mutator@entry=0x6fda60 <eval_const_expressions_mutator>,
    context=context@entry=0x4d1f010) at nodeFuncs.c:3298
#52155 0x00000000006fdab2 in eval_const_expressions_mutator (node=0x7dfa5b8,
    context=0x4d1f010) at clauses.c:3527
#52156 0x0000000000698ed8 in expression_tree_mutator (
    node=node@entry=0x7dfa8b8,
    mutator=mutator@entry=0x6fda60 <eval_const_expressions_mutator>,
    context=context@entry=0x4d1f010) at nodeFuncs.c:3165
#52157 0x00000000006fdab2 in eval_const_expressions_mutator (node=0x7dfa8b8,
    context=context@entry=0x4d1f010) at clauses.c:3527
#52158 0x00000000006ff204 in eval_const_expressions (
    root=root@entry=0x7df9a30, node=<optimized out>) at clauses.c:2107
#52159 0x00000000006e0d8e in preprocess_expression (
    root=root@entry=0x7df9a30, expr=<optimized out>, kind=kind@entry=1)
    at planner.c:1124
#52160 0x00000000006e93cd in subquery_planner (glob=glob@entry=0x7df8e08,
    parse=<optimized out>, parse@entry=0x770fe50,
    parent_root=parent_root@entry=0x0, hasRecursion=hasRecursion@entry=false,
    tuple_fraction=tuple_fraction@entry=0) at planner.c:791
#52161 0x00000000006e9f13 in standard_planner (parse=0x770fe50,
    query_string=<optimized out>, cursorOptions=2048,
    boundParams=<optimized out>) at planner.c:406
#52162 0x00000000007cedca in pg_plan_query (querytree=0x770fe50,
    query_string=0x770e650 "SELECT ST_SimplifyPreserveTopology(foo1.the_geom, 20.1)  As result\n\t\t\t\t\t\t\tFROM ((SELECT '0106000020E61000000100000001030000000100000005", '0' <repeats 18 times>, "F07F", '0' <repeats 12 times>, "F07F", '0' <repeats 12 times>, "F07F", '0' <repeats 11 times>...,
    cursorOptions=2048, boundParams=0x0) at postgres.c:883
#52163 0x00000000007ceed1 in pg_plan_queries (querytrees=0x7df99d8,
    query_string=query_string@entry=0x770e650 "SELECT ST_SimplifyPreserveTopology(foo1.the_geom, 20.1)  As result\n\t\t\t\t\t\t\tFROM ((SELECT '0106000020E61000000100000001030000000100000005", '0' <repeats 18 times>, "F07F", '0' <repeats 12 times>, "F07F", '0' <repeats 12 times>, "F07F", '0' <repeats 11 times>..., cursorOptions=cursorOptions@entry=2048, boundParams=boundParams@entry=0x0)
    at postgres.c:975
#52164 0x00000000007cf270 in exec_simple_query (
    query_string=0x770e650 "SELECT ST_SimplifyPreserveTopology(foo1.the_geom, 20.1)  As result\n\t\t\t\t\t\t\tFROM ((SELECT '0106000020E61000000100000001030000000100000005", '0' <repeats 18 times>, "F07F", '0' <repeats 12 times>, "F07F", '0' <repeats 12 times>, "F07F", '0' <repeats 11 times>...) at postgres.c:1169
#52165 0x00000000007d24a1 in PostgresMain (dbname=0x5b86fb0 "postgres",
    username=0x5b88e80 "postgres") at postgres.c:4581
#52166 0x00000000007355fd in BackendRun (port=0x4d1f7b0, port=0x4d1f7b0)
    at postmaster.c:4504
#52167 SubPostmasterMain (argc=argc@entry=3, argv=argv@entry=0x5b86db0)
    at postmaster.c:5008
#52168 0x000000000098ad1e in main (argc=3, argv=0x5b86db0) at main.c:194

[pramsey]

The question, as always for these kinds of things, is where to put the fix. We could put a big "no infinites" test at the PostGIS level, before calling into GEOS. Or we could put a test in Quadtree::Root to throw an exception in the case of infinite envelopes, since the Quadtree code is definitely not infinite-safe. I lean towards the latter as the "correct" fix, but it means this won't be fixed "in postgis", but in a patch release of GEOS.

[mdavis]

GEOS is generally not infinite-ordinate-safe. Is it going to be a whack-a-mole exercise to find all of them and add checks? If so, maybe it is best to check for Inf on the PG side, when creating GEOS geometries for passing to GEOS functions?

[robe]

I think the fix should eventually go into GEOS, but given it seems at moment we have only 3 functions in PostGIS impacted, seemed harmless enough to fix at the postgis level.

We do have ST_SetPoint #5319 impacted as well, which is not a GEOS function as I recall, so we still need an easy way to trap infinites on the postgis side.

[robe]

P.S. I didn't test SFCGAL, but I suspect we might have similar issues there too.

[pramsey]

Replying to mdavis:

GEOS is generally not infinite-ordinate-safe.

And yet Envelope says that it can sometimes hold infinite sided bounds! Gah!

I think in general having known infinite loops in GEOS is bad? Like, "feed this function an Inf and it'll lock up on you". So a GEOS patch is not a bad idea?

[mdavis]

Replying to pramsey:

I think in general having known infinite loops in GEOS is bad? Like, "feed this function an Inf and it'll lock up on you". So a GEOS patch is not a bad idea?

The bright light of a new day has clarified my thinking. Yes, I agree that GEOS should be fixed so that it does not crash for any inputs.

A fix in Quadtree is probably the minimum required, for this case. I wonder how to fix all the other potential cases of Inf causing crashes? It's tempting to put a check in Envelope creation… but maybe that's too pervasive.

For sure validity checking should be tightened up to report Inf ordinates as invalid.

[pramsey]

Fixed in GEOS at ​https://github.com/libgeos/geos/commit/13563b3c29a63f389794c2b6a4c94d9c6a0abf96

[Paul Ramsey <pramsey@…>]

In f2ae230a/git:

Prefilter to check for non-finite coordinates before feeding
ST_SimplifyPreserveTopology, to avoid crash/hang.
Closes #5320

[robe]

@pramsey you planning to backport this or just keeping for 3.4?

Also note the others #5315 (crash in ST_Buffer, reported by a user), #5318 (ST_MaximumInscribed circle ) and #5319 a non-geos function.

I can handle the remaining if you want, but would be nice to backport them I think.

I'd also like to put in my new changes to garden crasher that exercises this issue, but can't do that until the patches are complete since it will crash the berries.

[robe]

Still need to backport and add a test

[Regina Obe <lr@…>]

In bf77e55/git:

Prefilter to check for non-finite coordinates before feeding
ST_SimplifyPreserveTopology, to avoid crash/hang.
for PostGIS 3.3.3
Closes #5320

[Regina Obe <lr@…>]

In a57b2621/git:

Add test for PostGIS 3.4.0. References #5320

[Regina Obe <lr@…>]

In 0239850/git:

Prefilter to check for non-finite coordinates before feeding
ST_SimplifyPreserveTopology, ST_Buffer, ST_SetPoint,
ST_MinimumInscribedCircle
to avoid crash/hang.
References #5320
References #5315
References #5318
References #5319
for PostGIS 3.2.5

[Regina Obe <lr@…>]

In 11335bb/git:

Prefilter to check for non-finite coordinates before feeding
ST_SimplifyPreserveTopology, ST_Buffer, ST_SetPoint,
ST_MinimumInscribedCircle
to avoid crash/hang.
References #5320
References #5315
References #5318
References #5319
for PostGIS 3.1.9

[Regina Obe <lr@…>]

In 92e7399/git:

Prefilter to check for non-finite coordinates before feeding
ST_SimplifyPreserveTopology, ST_Buffer, ST_SetPoint
to avoid crash/hang.
References #5320
References #5315
References #5319
for PostGIS 3.0.9

[Regina Obe <lr@…>]

In 256dd5e/git:

Free lwgeom
References #5320, #5315, #5318
for PostGIS 3.4.0

[Regina Obe <lr@…>]

In 6885b9d/git:

Free lwgeom
References #5320, #5315, #5318
for PostGIS 3.3.3

[Regina Obe <lr@…>]

In 7c5bf6f/git:

Free lwgeom
Closes #5320
Closes #5315
Closes #5318
for PostGIS 3.2.5