Memory corruption in mvt_agg_transfn

[multun]

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.

[robe]

related pull request - ​https://git.osgeo.org/gitea/postgis/postgis/pulls/92

[Regina Obe <lr@…>]

In 8176e19e/git:

mvt: introduce feature_builder and fix a buffer overflow

The previous code tried to manually handle resizing the tags array
whenever used, but failed to keep it properly sized.

This commit adds an abstraction over unfinished features, which
enables centralized array size bookkeeping.

Closes ​https://git.osgeo.org/gitea/postgis/postgis/pulls/92 for PostGIS
3.3.0
Closes #5088 for PostGIS 3.3.0