[postgis-tickets] r16723 - Fix undefined behaviour in ptarray_segmentize2d

[Raul]

Author: algunenano
Date: 2018-09-07 04:19:22 -0700 (Fri, 07 Sep 2018)
New Revision: 16723

Modified:
   trunk/NEWS
   trunk/liblwgeom/cunit/cu_measures.c
   trunk/liblwgeom/ptarray.c
Log:
Fix undefined behaviour in ptarray_segmentize2d

Closes #4173
References #4153
Closes https://github.com/postgis/postgis/pull/292


Modified: trunk/NEWS
===================================================================
--- trunk/NEWS	2018-09-07 10:59:01 UTC (rev 16722)
+++ trunk/NEWS	2018-09-07 11:19:22 UTC (rev 16723)
@@ -8,6 +8,7 @@
   - #4163, MVT: Fix resource leak when the first geometry is NULL (Raúl Marín)
   - #4161, MVT: Drop geometries smaller than the resolution (Raúl Marín)
   - #4172, Fix memory leak in lwgeom_offsetcurve (Raúl Marín)
+  - #4173, Fix undefined behaviour in ptarray_segmentize2d (Raúl Marín)
 
 PostGIS 2.5.0rc1
 2018/08/19

Modified: trunk/liblwgeom/cunit/cu_measures.c
===================================================================
--- trunk/liblwgeom/cunit/cu_measures.c	2018-09-07 10:59:01 UTC (rev 16722)
+++ trunk/liblwgeom/cunit/cu_measures.c	2018-09-07 11:19:22 UTC (rev 16723)
@@ -527,17 +527,23 @@
 	lwgeom_free(linein);
 	lwgeom_free(lineout);
 
+	/* test too many segments */
+	linein = lwgeom_from_wkt("LINESTRING(0 0,10 0)", LW_PARSER_CHECK_NONE);
+	lineout = lwgeom_segmentize2d(linein, 1e-100);
+	CU_ASSERT_EQUAL(lineout, NULL);
+	lwgeom_free(linein);
+
 	/* test interruption */
 
 	linein = lwgeom_from_wkt("LINESTRING(0 0,10 0)", LW_PARSER_CHECK_NONE);
 	lwgeom_request_interrupt();
-	lineout = lwgeom_segmentize2d(linein, 1e-100);
+	lineout = lwgeom_segmentize2d(linein, INT32_MAX);
 	CU_ASSERT_EQUAL(lineout, NULL);
 	lwgeom_free(linein);
 
 	linein = lwgeom_from_wkt("MULTILINESTRING((0 0,10 0),(20 0, 30 0))", LW_PARSER_CHECK_NONE);
 	lwgeom_request_interrupt();
-	lineout = lwgeom_segmentize2d(linein, 1e-100);
+	lineout = lwgeom_segmentize2d(linein, INT32_MAX);
 	CU_ASSERT_EQUAL(lineout, NULL);
 	lwgeom_free(linein);
 
@@ -545,7 +551,7 @@
 	  "MULTIPOLYGON(((0 0,20 0,20 20,0 20,0 0),(2 2,2 4,4 4,4 2,2 2),(6 6,6 8,8 8,8 6,6 6)),((40 0,40 20,60 20,60 0,40 0),(42 2,42 4,44 4,44 2,42 2)))"
 	  , LW_PARSER_CHECK_NONE);
 	lwgeom_request_interrupt();
-	lineout = lwgeom_segmentize2d(linein, 1e-100);
+	lineout = lwgeom_segmentize2d(linein, INT32_MAX);
 	CU_ASSERT_EQUAL(lineout, NULL);
 	lwgeom_free(linein);
 
@@ -554,13 +560,15 @@
 	  , LW_PARSER_CHECK_NONE);
 	CU_ASSERT_FATAL(linein != NULL);
 	lwgeom_request_interrupt();
-	lineout = lwgeom_segmentize2d(linein, 1e-100);
+	lineout = lwgeom_segmentize2d(linein, INT32_MAX);
 	CU_ASSERT_EQUAL(lineout, NULL);
 	lwgeom_free(linein);
 
 	linein = lwgeom_from_wkt("LINESTRING(20 0, 30 0)", LW_PARSER_CHECK_NONE);
+	CU_ASSERT_FATAL(linein != NULL);
 	/* NOT INTERRUPTED */
 	lineout = lwgeom_segmentize2d(linein, 5);
+	CU_ASSERT_NOT_EQUAL_FATAL(lineout, NULL);
 	strout = lwgeom_to_ewkt(lineout);
 	ASSERT_STRING_EQUAL(strout, "LINESTRING(20 0,25 0,30 0)");
 	lwfree(strout);

Modified: trunk/liblwgeom/ptarray.c
===================================================================
--- trunk/liblwgeom/ptarray.c	2018-09-07 10:59:01 UTC (rev 16722)
+++ trunk/liblwgeom/ptarray.c	2018-09-07 11:19:22 UTC (rev 16723)
@@ -442,13 +442,24 @@
 		 * breaks those "strict" rules.
 		 */
 		POINT4D *p1ptr=&p1, *p2ptr=&p2;
+		double segments;
 
 		getPoint4d_p(ipa, i, &p2);
 
 		segdist = distance2d_pt_pt((POINT2D *)p1ptr, (POINT2D *)p2ptr);
 		/* Split input segment into shorter even chunks */
-		nseg = ceil(segdist / dist);
+		segments = ceil(segdist / dist);
 
+		/* Uses INT32_MAX instead of UINT32_MAX to be safe that it fits */
+		if (segments >= INT32_MAX)
+		{
+			lwnotice("%s:%d - %s: Too many segments required (%e)",
+				__FILE__, __LINE__,__func__, segments);
+			ptarray_free(opa);
+			return NULL;
+		}
+		nseg = segments;
+
 		for (j = 1; j < nseg; j++)
 		{
 			pbuf.x = p1.x + (p2.x - p1.x) * j / nseg;