[postgis-tickets] r17904 - WKB: Avoid buffer overflow
Raul
raul at rmr.ninja
Thu Oct 10 09:58:46 PDT 2019
Author: algunenano
Date: 2019-10-10 09:58:46 -0700 (Thu, 10 Oct 2019)
New Revision: 17904
Modified:
trunk/NEWS
trunk/liblwgeom/cunit/cu_in_wkb.c
trunk/liblwgeom/lwin_wkb.c
Log:
WKB: Avoid buffer overflow
This only happens when not running under PG context, as
lwerror continues execution and that means that even after
detecting there isn't enough bytes still try to read from the buffer
Closes #4535
Closes https://github.com/postgis/postgis/pull/495
Modified: trunk/NEWS
===================================================================
--- trunk/NEWS 2019-10-10 13:02:38 UTC (rev 17903)
+++ trunk/NEWS 2019-10-10 16:58:46 UTC (rev 17904)
@@ -10,6 +10,7 @@
- #4534, Fix leak in lwcurvepoly_from_wkb_state (Raúl Marín)
- #4536, Fix leak in lwcollection_from_wkb_state (Raúl Marín)
- #4537, Fix leak in WKT collection parser (Raúl Marín)
+ - #4535, WKB: Avoid buffer overflow (Raúl Marín)
PostGIS 3.0.0rc1
2019/10/08
Modified: trunk/liblwgeom/cunit/cu_in_wkb.c
===================================================================
--- trunk/liblwgeom/cunit/cu_in_wkb.c 2019-10-10 13:02:38 UTC (rev 17903)
+++ trunk/liblwgeom/cunit/cu_in_wkb.c 2019-10-10 16:58:46 UTC (rev 17904)
@@ -228,7 +228,7 @@
}
static void
-test_wkb_leak(void)
+test_wkb_fuzz(void)
{
/* OSS-FUZZ https://trac.osgeo.org/postgis/ticket/4534 */
uint8_t wkb[36] = {000, 000, 000, 000, 015, 000, 000, 000, 003, 000, 200, 000, 000, 010, 000, 000, 000, 000,
@@ -256,6 +256,11 @@
001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001, 001};
g = lwgeom_from_wkb(wkb2, 319, LW_PARSER_CHECK_NONE);
lwgeom_free(g);
+
+ /* OSS-FUZZ: https://trac.osgeo.org/postgis/ticket/4535 */
+ uint8_t wkb3[9] = {0x01, 0x03, 0x00, 0x00, 0x10, 0x8d, 0x55, 0xf3, 0xff};
+ g = lwgeom_from_wkb(wkb3, 9, LW_PARSER_CHECK_NONE);
+ lwgeom_free(g);
}
/*
@@ -278,5 +283,5 @@
PG_ADD_TEST(suite, test_wkb_in_multicurve);
PG_ADD_TEST(suite, test_wkb_in_multisurface);
PG_ADD_TEST(suite, test_wkb_in_malformed);
- PG_ADD_TEST(suite, test_wkb_leak);
+ PG_ADD_TEST(suite, test_wkb_fuzz);
}
Modified: trunk/liblwgeom/lwin_wkb.c
===================================================================
--- trunk/liblwgeom/lwin_wkb.c 2019-10-10 13:02:38 UTC (rev 17903)
+++ trunk/liblwgeom/lwin_wkb.c 2019-10-10 16:58:46 UTC (rev 17904)
@@ -36,14 +36,15 @@
typedef struct
{
const uint8_t *wkb; /* Points to start of WKB */
+ int32_t srid; /* Current SRID we are handling */
size_t wkb_size; /* Expected size of WKB */
- int swap_bytes; /* Do an endian flip? */
- int check; /* Simple validity checks on geometries */
- uint32_t lwtype; /* Current type we are handling */
- int32_t srid; /* Current SRID we are handling */
- int has_z; /* Z? */
- int has_m; /* M? */
- int has_srid; /* SRID? */
+ int8_t swap_bytes; /* Do an endian flip? */
+ int8_t check; /* Simple validity checks on geometries */
+ int8_t lwtype; /* Current type we are handling */
+ int8_t has_z; /* Z? */
+ int8_t has_m; /* M? */
+ int8_t has_srid; /* SRID? */
+ int8_t error; /* An error was found (not enough bytes to read) */
const uint8_t *pos; /* Current parse position */
} wkb_parse_state;
@@ -128,7 +129,7 @@
if( (s->pos + next) > (s->wkb + s->wkb_size) )
{
lwerror("WKB structure does not match expected size!");
- return;
+ s->error = LW_TRUE;
}
}
@@ -255,6 +256,8 @@
LWDEBUG(4, "Entered function");
wkb_parse_state_check(s, WKB_BYTE_SIZE);
+ if (s->error)
+ return 0;
LWDEBUG(4, "Passed state check");
char_value = s->pos[0];
@@ -273,6 +276,8 @@
uint32_t i = 0;
wkb_parse_state_check(s, WKB_INT_SIZE);
+ if (s->error)
+ return 0;
memcpy(&i, s->pos, WKB_INT_SIZE);
@@ -302,8 +307,6 @@
{
double d = 0;
- wkb_parse_state_check(s, WKB_DOUBLE_SIZE);
-
memcpy(&d, s->pos, WKB_DOUBLE_SIZE);
/* Swap? Copy into a stack-allocated integer. */
@@ -340,6 +343,8 @@
/* Calculate the size of this point array. */
npoints = integer_from_wkb_state(s);
+ if (s->error)
+ return NULL;
if (npoints > maxpoints)
{
lwerror("Pointarray length (%d) is too large");
@@ -358,6 +363,8 @@
/* Does the data we want to read exist? */
wkb_parse_state_check(s, pa_size);
+ if (s->error)
+ return NULL;
/* If we're in a native endianness, we can just copy the data directly! */
if( ! s->swap_bytes )
@@ -405,6 +412,8 @@
/* Does the data we want to read exist? */
wkb_parse_state_check(s, pa_size);
+ if (s->error)
+ return NULL;
/* If we're in a native endianness, we can just copy the data directly! */
if( ! s->swap_bytes )
@@ -449,10 +458,13 @@
static LWLINE* lwline_from_wkb_state(wkb_parse_state *s)
{
POINTARRAY *pa = ptarray_from_wkb_state(s);
+ if (s->error)
+ return NULL;
if( pa == NULL || pa->npoints == 0 )
{
- ptarray_free(pa);
+ if (pa)
+ ptarray_free(pa);
return lwline_construct_empty(s->srid, s->has_z, s->has_m);
}
@@ -477,9 +489,15 @@
static LWCIRCSTRING* lwcircstring_from_wkb_state(wkb_parse_state *s)
{
POINTARRAY *pa = ptarray_from_wkb_state(s);
+ if (s->error)
+ return NULL;
if( pa == NULL || pa->npoints == 0 )
+ {
+ if (pa)
+ ptarray_free(pa);
return lwcircstring_construct_empty(s->srid, s->has_z, s->has_m);
+ }
if( s->check & LW_PARSER_CHECK_MINPOINTS && pa->npoints < 3 )
{
@@ -507,6 +525,8 @@
static LWPOLY* lwpoly_from_wkb_state(wkb_parse_state *s)
{
uint32_t nrings = integer_from_wkb_state(s);
+ if (s->error)
+ return NULL;
uint32_t i = 0;
LWPOLY *poly = lwpoly_construct_empty(s->srid, s->has_z, s->has_m);
@@ -519,12 +539,13 @@
for( i = 0; i < nrings; i++ )
{
POINTARRAY *pa = ptarray_from_wkb_state(s);
- if( pa == NULL )
- continue;
+ if (pa == NULL)
+ return NULL;
/* Check for at least four points. */
- if( s->check & LW_PARSER_CHECK_MINPOINTS && pa->npoints < 4 )
+ if (s->check & LW_PARSER_CHECK_MINPOINTS && pa->npoints < 4)
{
+ lwpoly_free(poly);
LWDEBUGF(2, "%s must have at least four points in each ring", lwtype_name(s->lwtype));
lwerror("%s must have at least four points in each ring", lwtype_name(s->lwtype));
return NULL;
@@ -533,6 +554,7 @@
/* Check that first and last points are the same. */
if( s->check & LW_PARSER_CHECK_CLOSURE && ! ptarray_is_closed_2d(pa) )
{
+ lwpoly_free(poly);
LWDEBUGF(2, "%s must have closed rings", lwtype_name(s->lwtype));
lwerror("%s must have closed rings", lwtype_name(s->lwtype));
return NULL;
@@ -541,8 +563,10 @@
/* Add ring to polygon */
if ( lwpoly_add_ring(poly, pa) == LW_FAILURE )
{
+ lwpoly_free(poly);
LWDEBUG(2, "Unable to add ring to polygon");
lwerror("Unable to add ring to polygon");
+ return NULL;
}
}
@@ -560,6 +584,8 @@
static LWTRIANGLE* lwtriangle_from_wkb_state(wkb_parse_state *s)
{
uint32_t nrings = integer_from_wkb_state(s);
+ if (s->error)
+ return NULL;
LWTRIANGLE *tri = lwtriangle_construct_empty(s->srid, s->has_z, s->has_m);
POINTARRAY *pa = NULL;
@@ -606,6 +632,8 @@
static LWCURVEPOLY* lwcurvepoly_from_wkb_state(wkb_parse_state *s)
{
uint32_t ngeoms = integer_from_wkb_state(s);
+ if (s->error)
+ return NULL;
LWCURVEPOLY *cp = lwcurvepoly_construct_empty(s->srid, s->has_z, s->has_m);
LWGEOM *geom = NULL;
uint32_t i;
@@ -641,6 +669,8 @@
static LWCOLLECTION* lwcollection_from_wkb_state(wkb_parse_state *s)
{
uint32_t ngeoms = integer_from_wkb_state(s);
+ if (s->error)
+ return NULL;
LWCOLLECTION *col = lwcollection_construct_empty(s->lwtype, s->srid, s->has_z, s->has_m);
LWGEOM *geom = NULL;
uint32_t i;
@@ -687,6 +717,8 @@
/* Fail when handed incorrect starting byte */
wkb_little_endian = byte_from_wkb_state(s);
+ if (s->error)
+ return NULL;
if( wkb_little_endian != 1 && wkb_little_endian != 0 )
{
LWDEBUG(4,"Leaving due to bad first byte!");
@@ -706,6 +738,8 @@
/* Read the type number */
wkb_type = integer_from_wkb_state(s);
+ if (s->error)
+ return NULL;
LWDEBUGF(4,"Got WKB type number: 0x%X", wkb_type);
lwtype_from_wkb_state(s, wkb_type);
@@ -713,6 +747,8 @@
if( s->has_srid )
{
s->srid = clamp_srid(integer_from_wkb_state(s));
+ if (s->error)
+ return NULL;
/* TODO: warn on explicit UNKNOWN srid ? */
LWDEBUGF(4,"Got SRID: %u", s->srid);
}
@@ -785,8 +821,12 @@
s.has_z = LW_FALSE;
s.has_m = LW_FALSE;
s.has_srid = LW_FALSE;
+ s.error = LW_FALSE;
s.pos = wkb;
+ if (!wkb || !wkb_size)
+ return NULL;
+
return lwgeom_from_wkb_state(&s);
}
More information about the postgis-tickets
mailing list