From 24e8cdf11029d02599777743faf19521c7d0ae7a Mon Sep 17 00:00:00 2001 From: Harsh Raj Singhania <40535627+HarshRajSinghania@users.noreply.github.com> Date: Fri, 25 Sep 2026 23:51:51 +0530 Subject: [PATCH] Fix dimensional pcpatch WKB bounds validation --- lib/cunit/cu_pc_bytes.c | 33 ++++++++++++++++++++++++++++++++- lib/cunit/cu_pc_patch.c | 24 +++++++++++++++++++++++- lib/pc_api.h | 5 +++-- lib/pc_bytes.c | 22 +++++++++++++++++++--- lib/pc_patch.c | 9 +++++++-- lib/pc_patch_dimensional.c | 36 ++++++++++++++++++++++++++++++++---- 6 files changed, 116 insertions(+), 13 deletions(-) diff --git a/lib/cunit/cu_pc_bytes.c b/lib/cunit/cu_pc_bytes.c index b70e7d54..24ba27ea 100644 --- a/lib/cunit/cu_pc_bytes.c +++ b/lib/cunit/cu_pc_bytes.c @@ -516,12 +516,43 @@ static void test_uncompressed_filter() // pc_bytes_free(epcb); } +static void test_deserialize_bounds() +{ + uint8_t buf[13] = {0}; + int32_t claimed = 65536; + PCBYTES pcb = {0}; + PCDIMENSION dim = {0}; + int rv; + + memcpy(buf + 1, &claimed, sizeof(claimed)); + memset(buf + 5, 0x41, 8); + + rv = pc_bytes_deserialize(buf, sizeof(buf), &dim, &pcb, PC_FALSE, 0); + + CU_ASSERT_EQUAL(rv, PC_FAILURE); + CU_ASSERT_EQUAL(pcb.bytes, NULL); +} + +static void test_deserialize_short_header() +{ + uint8_t buf[4] = {0}; + PCBYTES pcb = {0}; + PCDIMENSION dim = {0}; + int rv; + + rv = pc_bytes_deserialize(buf, sizeof(buf), &dim, &pcb, PC_FALSE, 0); + + CU_ASSERT_EQUAL(rv, PC_FAILURE); + CU_ASSERT_EQUAL(pcb.bytes, NULL); +} + /* REGISTER ***********************************************************/ CU_TestInfo bytes_tests[] = { PC_TEST(test_run_length_encoding), PC_TEST(test_sigbits_encoding), PC_TEST(test_zlib_encoding), PC_TEST(test_rle_filter), - PC_TEST(test_uncompressed_filter), CU_TEST_INFO_NULL}; + PC_TEST(test_uncompressed_filter), PC_TEST(test_deserialize_bounds), + PC_TEST(test_deserialize_short_header), CU_TEST_INFO_NULL}; CU_SuiteInfo bytes_suite = {.pName = "bytes", .pInitFunc = init_suite, diff --git a/lib/cunit/cu_pc_patch.c b/lib/cunit/cu_pc_patch.c index da90b7b4..09349a3f 100644 --- a/lib/cunit/cu_pc_patch.c +++ b/lib/cunit/cu_pc_patch.c @@ -1314,6 +1314,28 @@ static void test_patch_transform_compression_none() pc_pointlist_free(pl); } +static void test_patch_dimensional_wkb_bounds() +{ + uint8_t wkb[18] = {0}; + uint32_t pcid = 0; + uint32_t compression = PC_DIMENSIONAL; + uint32_t npoints = 1; + uint8_t dim_compression = PC_DIM_NONE; + int32_t claimed_size = 65536; + PCPATCH *patch; + + wkb[0] = machine_endian(); + memcpy(wkb + 1, &pcid, sizeof(pcid)); + memcpy(wkb + 5, &compression, sizeof(compression)); + memcpy(wkb + 9, &npoints, sizeof(npoints)); + wkb[13] = dim_compression; + memcpy(wkb + 14, &claimed_size, sizeof(claimed_size)); + + patch = pc_patch_from_wkb(simpleschema, wkb, sizeof(wkb)); + + CU_ASSERT_EQUAL(patch, NULL); +} + /* REGISTER ***********************************************************/ CU_TestInfo patch_tests[] = { @@ -1359,7 +1381,7 @@ CU_TestInfo patch_tests[] = { PC_TEST(test_patch_set_schema_compression_lazperf), #endif PC_TEST(test_patch_transform_compression_none), - CU_TEST_INFO_NULL}; + PC_TEST(test_patch_dimensional_wkb_bounds), CU_TEST_INFO_NULL}; CU_SuiteInfo patch_suite = {.pName = "patch", .pInitFunc = init_suite, diff --git a/lib/pc_api.h b/lib/pc_api.h index 8395405b..b52cefcd 100644 --- a/lib/pc_api.h +++ b/lib/pc_api.h @@ -415,8 +415,9 @@ size_t pc_bytes_serialized_size(const PCBYTES *pcb); int pc_bytes_serialize(const PCBYTES *pcb, uint8_t *buf, size_t *size); /** Read a buffer up into a bytes structure */ -int pc_bytes_deserialize(const uint8_t *buf, const PCDIMENSION *dim, - PCBYTES *pcb, int readonly, int flip_endian); +int pc_bytes_deserialize(const uint8_t *buf, size_t bufsize, + const PCDIMENSION *dim, PCBYTES *pcb, int readonly, + int flip_endian); /** Wrap serialized stats in a new stats objects */ PCSTATS *pc_stats_new_from_data(const PCSCHEMA *schema, const uint8_t *mindata, diff --git a/lib/pc_bytes.c b/lib/pc_bytes.c index c4d94af3..6938621e 100644 --- a/lib/pc_bytes.c +++ b/lib/pc_bytes.c @@ -1344,11 +1344,27 @@ int pc_bytes_serialize(const PCBYTES *pcb, uint8_t *buf, size_t *size) return PC_SUCCESS; } -int pc_bytes_deserialize(const uint8_t *buf, const PCDIMENSION *dim, - PCBYTES *pcb, int readonly, int flip_endian) +int pc_bytes_deserialize(const uint8_t *buf, size_t bufsize, + const PCDIMENSION *dim, PCBYTES *pcb, int readonly, + int flip_endian) { + int32_t size; + + if (bufsize < 5) + { + pcerror("%s: truncated dimension header", __func__); + return PC_FAILURE; + } + pcb->compression = buf[0]; - pcb->size = wkb_get_int32(buf + 1, flip_endian); + size = wkb_get_int32(buf + 1, flip_endian); + if (size < 0 || (size_t)size > bufsize - 5) + { + pcerror("%s: dimension size exceeds remaining WKB buffer", __func__); + return PC_FAILURE; + } + + pcb->size = (size_t)size; pcb->readonly = readonly; if (readonly && flip_endian) pcerror("pc_bytes_deserialize: cannot create a read-only buffer on " diff --git a/lib/pc_patch.c b/lib/pc_patch.c index 8d28356e..512530ea 100644 --- a/lib/pc_patch.c +++ b/lib/pc_patch.c @@ -258,10 +258,12 @@ PCPATCH *pc_patch_from_wkb(const PCSCHEMA *s, uint8_t *wkb, size_t wkbsize) */ uint32_t compression, pcid; PCPATCH *patch; + static size_t hdrsz = 1 + 4 + 4 + 4; - if (!wkbsize) + if (wkbsize < hdrsz) { - pcerror("%s: zero length wkb", __func__); + pcerror("%s: truncated WKB header", __func__); + return NULL; } /* @@ -303,6 +305,9 @@ PCPATCH *pc_patch_from_wkb(const PCSCHEMA *s, uint8_t *wkb, size_t wkbsize) } } + if (!patch) + return NULL; + if (PC_FAILURE == pc_patch_compute_extent(patch)) pcerror("%s: pc_patch_compute_extent failed", __func__); diff --git a/lib/pc_patch_dimensional.c b/lib/pc_patch_dimensional.c index 186a65c7..d608c454 100644 --- a/lib/pc_patch_dimensional.c +++ b/lib/pc_patch_dimensional.c @@ -263,18 +263,27 @@ PCPATCH *pc_patch_dimensional_from_wkb(const PCSCHEMA *schema, /* byte: endianness (1 = NDR, 0 = XDR) uint32: pcid (key to POINTCLOUD_SCHEMAS) - uint32: compression (0 = no compression, 1 = dimensional, 2 = lazperf) + uint32: compression (0 = none, 1 = dimensional, 2 = lazperf) uint32: npoints dimensions[]: dims (interpret relative to pcid and compressions) */ static size_t hdrsz = 1 + 4 + 4 + 4; /* endian + pcid + compression + npoints */ PCPATCH_DIMENSIONAL *patch; - uint8_t swap_endian = (wkb[0] != machine_endian()); + uint8_t swap_endian; uint32_t npoints, ndims; const uint8_t *buf; + size_t remaining; int i; + if (wkbsize < hdrsz) + { + pcerror("%s: truncated WKB header", __func__); + return NULL; + } + + swap_endian = (wkb[0] != machine_endian()); + if (wkb_get_compression(wkb) != PC_DIMENSIONAL) { pcerror("%s: call with wkb that is not dimensionally compressed", __func__); @@ -293,13 +302,32 @@ PCPATCH *pc_patch_dimensional_from_wkb(const PCSCHEMA *schema, patch->stats = NULL; buf = wkb + hdrsz; + remaining = wkbsize - hdrsz; for (i = 0; i < ndims; i++) { PCBYTES *pcb = &(patch->bytes[i]); PCDIMENSION *dim = schema->dims[i]; - pc_bytes_deserialize(buf, dim, pcb, PC_FALSE /*readonly*/, swap_endian); + size_t serialized_size; + + if (PC_FAILURE == + pc_bytes_deserialize(buf, remaining, dim, pcb, PC_FALSE /*readonly*/, + swap_endian)) + { + pc_patch_dimensional_free(patch); + return NULL; + } + pcb->npoints = npoints; - buf += pc_bytes_serialized_size(pcb); + serialized_size = pc_bytes_serialized_size(pcb); + buf += serialized_size; + remaining -= serialized_size; + } + + if (remaining != 0) + { + pcerror("%s: unexpected trailing data in WKB", __func__); + pc_patch_dimensional_free(patch); + return NULL; } return (PCPATCH *)patch;