From 0f15c09b2564d81434ed5e3cc6c5aed427656ec0 Mon Sep 17 00:00:00 2001 From: Samuel Freilich Date: Wed, 16 Apr 2025 16:20:33 -0400 Subject: [PATCH] Fail if coords are outside of a safe range The computation of sentinel edges in InitEdgeDict potentially overflows if coordinates are more than half of float max/min, and it distinguishes between real coord bounds and those sentinel edges by a 0.01 margin, which is not distinguishable at that magnitude due to loss-of-precision. That can cause assert failures in debug mode or uninitialized reads or crashes in optimized mode. Instead, fail if coordinates are not in `[-2^32, 2^32]`. This defines TESS_MAX_VALUE and TESS_MIN_VALUE in the public headers as those bounds, in case clients want to clamp or otherwise sanitize invalid inputs. --- Include/tesselator.h | 8 +++++++ Source/tess.c | 9 +++++++- Tests/libtess2_test.cc | 52 +++++++++++++++++++++++++++++++----------- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/Include/tesselator.h b/Include/tesselator.h index 3d43155..8993daa 100755 --- a/Include/tesselator.h +++ b/Include/tesselator.h @@ -138,6 +138,14 @@ typedef struct TESSalloc TESSalloc; #define TESS_NOTUSED(v) do { (void)(1 ? (void)0 : ( (void)(v) ) ); } while(0) +// These two constants define the valid input coordinate range the library is +// able to operate on. Tesselation will fail if any of the coordinates are not +// within this range. Clients are responsible for dealing with inputs outside of +// this range (e.g. clamping or filtering invalid points, scaling down the +// coordinate space). +#define TESS_MAX_VALID_INPUT_VALUE ((TESSreal) (1<<23)) +#define TESS_MIN_VALID_INPUT_VALUE (-TESS_MAX_VALID_INPUT_VALUE) + // Custom memory allocator interface. // The internal memory allocator allocates mesh edges, vertices and faces // as well as dictionary nodes and active regions in buckets and uses simple diff --git a/Source/tess.c b/Source/tess.c index 2a3b96f..7525fd6 100755 --- a/Source/tess.c +++ b/Source/tess.c @@ -918,6 +918,11 @@ void OutputContours( TESStesselator *tess, TESSmesh *mesh, int vertexSize ) } } +int IsValidCoord(TESSreal coord) { + return coord <= TESS_MAX_VALID_INPUT_VALUE && + coord >= TESS_MIN_VALID_INPUT_VALUE; +} + void tessAddContour( TESStesselator *tess, int size, const void* vertices, int stride, int numVertices ) { @@ -942,7 +947,9 @@ void tessAddContour( TESStesselator *tess, int size, const void* vertices, { const TESSreal* coords = (const TESSreal*)src; src += stride; - if (isnan(coords[0]) || isnan(coords[1]) || (size > 2 && isnan(coords[2]))) { + if (!IsValidCoord(coords[0]) || + !IsValidCoord(coords[1]) || + (size > 2 && !IsValidCoord(coords[2]))) { // "Out of memory" isn't quite right, but give up and bail out tess->outOfMemory = 1; return; diff --git a/Tests/libtess2_test.cc b/Tests/libtess2_test.cc index 5039128..6d40934 100644 --- a/Tests/libtess2_test.cc +++ b/Tests/libtess2_test.cc @@ -167,10 +167,9 @@ TEST_F(Libtess2Test, FloatOverflowQuad) { {kFloatMin, kFloatMax}, {kFloatMax, kFloatMax}, {kFloatMax, kFloatMin}}); - EXPECT_NE(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, kNumTriangleVertices, kComponentCount, nullptr), 0); - EXPECT_EQ(tessGetElementCount(tess), 2); } TEST_F(Libtess2Test, SingularityQuad) { @@ -188,34 +187,30 @@ TEST_F(Libtess2Test, DegenerateQuad) { {0.64113313f, -1.f}, {-0.f, -0.f}, {-3.40282347e+38f, 1.f}}); - EXPECT_NE(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, kNumTriangleVertices, kComponentCount, nullptr), 0); - EXPECT_EQ(tessGetElementCount(tess), 2); } TEST_F(Libtess2Test, WidthOverflowsTri) { AddPolyline(tess, {{-2e+38f, 0}, {0, 0}, {2e+38f, -1}}); - EXPECT_NE(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, kNumTriangleVertices, kComponentCount, nullptr), 0); - EXPECT_EQ(tessGetElementCount(tess), 1); } TEST_F(Libtess2Test, HeightOverflowsTri) { AddPolyline(tess, {{0, 0}, {0, 2e+38f}, {-1, -2e+38f}}); - EXPECT_NE(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, kNumTriangleVertices, kComponentCount, nullptr), 0); - EXPECT_EQ(tessGetElementCount(tess), 1); } TEST_F(Libtess2Test, AreaOverflowsTri) { AddPolyline(tess, {{-2e+37f, 0.f}, {0, 5}, {1e37f, -5}}); - EXPECT_NE(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, kNumTriangleVertices, kComponentCount, nullptr), 0); - EXPECT_EQ(tessGetElementCount(tess), 1); } TEST_F(Libtess2Test, NanQuad) { @@ -229,7 +224,7 @@ TEST_F(Libtess2Test, NanQuad) { EXPECT_EQ(tessGetElementCount(tess), 0); } -TEST_F(Libtess2Test, DegenerateSquiggle) { +TEST_F(Libtess2Test, AvoidsCrashWhileFindingIntersection) { // Previously, this failed an assert while finding an intersection because // that fell back to taking a midpoint between two coordinates in a way that // could get the wrong answer because of the sum overflowing max float. @@ -261,10 +256,41 @@ TEST_F(Libtess2Test, DegenerateSquiggle) { {-1.f, 3.40282347e+38f}, {-0.f, 0.34419331f}, {1.f, 1.f}}); - EXPECT_NE(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, + kNumTriangleVertices, kComponentCount, nullptr), + 0); +} + +TEST_F(Libtess2Test, AvoidsCrashInAddRightEdges) { + AddPolyline(tess, {{{-0.5f, 1.f}, + {3.40282347e+38f, 0.f}, + {0.349171013f, 1.f}, + {1.f, 0.f}, + {1.f, -0.f}, + {0.594775498f, -0.f}, + {0.f, -0.f}, + {-0.f, 1.f}, + {0.f, 1.f}, + {2.20929384f, 1.f}, + {1.f, 1.f}, + {-0.f, -0.f}, + {3.40282347e+38f, -0.f}, + {-1.f, 0.f}, + {1.70141173e+38f, 0.391036272f}, + {3.40282347e+38f, 0.371295959f}, + {3.40282347e+38f, -0.f}, + {0.f, 0.234747186f}, + {-1.f, 1.f}, + {-1.f, -0.f}, + {3.40282347e+38f, 1.f}, + {-0.f, -0.f}, + {3.40282347e+38f, 1.f}, + {0.434241712f, 0.f}, + {1.f, 0.211511821f}, + {3.40282347e+38f, 1.f}}}); + EXPECT_EQ(tessTesselate(tess, TESS_WINDING_POSITIVE, TESS_POLYGONS, kNumTriangleVertices, kComponentCount, nullptr), 0); - EXPECT_EQ(tessGetElementCount(tess), 13); } } // namespace