[postgis-tickets] [PostGIS] #5088: Memory corruption in mvt_agg_transfn

[PostGIS]

#5088: Memory corruption in mvt_agg_transfn
---------------------+---------------------------
 Reporter:  multun   |      Owner:  pramsey
     Type:  defect   |     Status:  new
 Priority:  high     |  Milestone:  PostGIS 3.3.0
Component:  postgis  |    Version:  3.2.x
 Keywords:           |
---------------------+---------------------------
 Hello!

 We've had frequent crashes in mvt_agg_transfn, with this message: `could
 not find block containing chunk`. I thought it could be a buffer overflow,
 compiled postgis and postgres with debug and valgrind support enabled, and
 sure enough:

 {{{
 ==26== Invalid write of size 4
 ==26==    at 0x10B244C6: add_value_as_string_with_size (mvt.c:481)
 ==26==    by 0x10B295C6: add_value_as_string (mvt.c:490)
 ==26==    by 0x10B295C6: parse_jsonb (mvt.c:551)
 ==26==    by 0x10B295C6: parse_values (mvt.c:686)
 ==26==    by 0x10B295C6: mvt_agg_transfn (mvt.c:1052)
 ==26==    by 0x10B2BA47: pgis_asmvt_transfn (lwgeom_out_mvt.c:172)
 ==26==    by 0x4840EA: ExecAggPlainTransByVal (execExprInterp.c:4298)
 ==26==    by 0x47E974: ExecInterpExpr (execExprInterp.c:1696)
 ==26==    by 0x47ED9E: ExecInterpExprStillValid (execExprInterp.c:1807)
 ==26==    by 0x4A3F2F: ExecEvalExprSwitchContext (executor.h:322)
 ==26==    by 0x4A49E3: advance_aggregates (nodeAgg.c:850)
 ==26==    by 0x4A79AA: agg_retrieve_direct (nodeAgg.c:2462)
 ==26==    by 0x4A7417: ExecAgg (nodeAgg.c:2187)
 ==26==    by 0x4948C1: ExecProcNodeFirst (execProcnode.c:450)
 ==26==    by 0x4884F2: ExecProcNode (executor.h:248)
 ==26==  Address 0x1086e418 is 5,608 bytes inside a block of size 8,192
 alloc'd
 ==26==    at 0x48A26D9: malloc (vg_replace_malloc.c:380)
 ==26==    by 0x8A67B2: AllocSetContextCreateInternal (aset.c:468)
 ==26==    by 0x10B2BB55: pgis_asmvt_transfn (lwgeom_out_mvt.c:158)
 ==26==    by 0x4840EA: ExecAggPlainTransByVal (execExprInterp.c:4298)
 ==26==    by 0x47E974: ExecInterpExpr (execExprInterp.c:1696)
 ==26==    by 0x47ED9E: ExecInterpExprStillValid (execExprInterp.c:1807)
 ==26==    by 0x4A3F2F: ExecEvalExprSwitchContext (executor.h:322)
 ==26==    by 0x4A49E3: advance_aggregates (nodeAgg.c:850)
 ==26==    by 0x4A79AA: agg_retrieve_direct (nodeAgg.c:2462)
 ==26==    by 0x4A7417: ExecAgg (nodeAgg.c:2187)
 ==26==    by 0x4948C1: ExecProcNodeFirst (execProcnode.c:450)
 ==26==    by 0x4884F2: ExecProcNode (executor.h:248)
 ==26==
 }}}


 In this specific instance, when the overflow occurs on line 481 of mvt.c:

 `tags[ctx->row_columns * 2] = k`

 `tags` has room for 10 items (I deduced this from allocation metadata
 headers), yet `ctx->row_columns` is 5.

 So the tags array is too small. It is initially allocated with the number
 of already known hash keys as an initial size: `uint32_t *tags =
 palloc(ctx->keys_hash_i * 2 * sizeof(*tags));`
 It may very well be a good guess, yet nothing ensures the array is big
 enough with properties are added to the feature.

 The array is only resized when a new key is met in parse_jsonb, which does
 not seem to be enough.

-- 
Ticket URL: <https://trac.osgeo.org/postgis/ticket/5088>
PostGIS <http://trac.osgeo.org/postgis/>
The PostGIS Trac is used for bug, enhancement & task tracking, a user and developer wiki, and a view into the subversion code repository of PostGIS project.