Fix heap out-of-bounds read in dimensional pcpatch WKB parsing

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,

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,

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,

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 "

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__);
 

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;