Fix heap buffer overflow in NDCELL decompression path

plugins/filters/ndcell/CMakeLists.txt

@@ -5,17 +5,24 @@ if(BUILD_TESTS)
 
     # targets
     add_executable(test_ndcell test_ndcell.c)
+    add_executable(test_ndcell_oversized_blockshape test_ndcell_oversized_blockshape.c)
     # Define the BLOSC_TESTING symbol so normally-hidden functions
     # aren't hidden from the view of the test programs.
     set_property(
             TARGET test_ndcell
             APPEND PROPERTY COMPILE_DEFINITIONS BLOSC_TESTING)
+    set_property(
+            TARGET test_ndcell_oversized_blockshape
+            APPEND PROPERTY COMPILE_DEFINITIONS BLOSC_TESTING)
 
     target_link_libraries(test_ndcell blosc_testing)
+    target_link_libraries(test_ndcell_oversized_blockshape blosc_testing)
 
     # tests
     add_test(NAME test_plugin_test_ndcell
         COMMAND ${CMAKE_CROSSCOMPILING_EMULATOR} $<TARGET_FILE:test_ndcell>)
+    add_test(NAME test_plugin_test_ndcell_oversized_blockshape
+        COMMAND ${CMAKE_CROSSCOMPILING_EMULATOR} $<TARGET_FILE:test_ndcell_oversized_blockshape>)
 
     # Copy test files
     file(GLOB TESTS_DATA ../../test_data/*.b2nd)

plugins/filters/ndcell/ndcell.c

@@ -48,27 +48,79 @@ int ndcell_forward(const uint8_t *input, uint8_t *output, int32_t length, uint8_
   }
   int typesize = cparams->typesize;
 
-  int8_t cell_shape = (int8_t) meta;
-  const int cell_size = (int) pow(cell_shape, ndim);
+  // `meta` is the cell side length and comes from the chunk header (uint8_t,
+  // 0..255).  Interpret it as unsigned and reject 0: an (int8_t) cast would turn
+  // 128..255 into negative values and 0 would later divide by zero in
+  // `(blockshape[i] + cell_shape - 1) / cell_shape`.
+  int cell_shape = (int) meta;
+  if (cell_shape <= 0) {
+    free(shape);
+    free(chunkshape);
+    free(blockshape);
+    BLOSC_TRACE_ERROR("Invalid cell_shape %d", cell_shape);
+    return BLOSC2_ERROR_FAILURE;
+  }
+  // cell_size is cell_shape^ndim (elements per cell).  Compute it with checked
+  // 64-bit multiplication: an (int) pow() can overflow / lose precision, and
+  // casting an out-of-range double back to int is undefined behaviour that could
+  // bypass the "buffer smaller than cell" check further down.
+  int64_t cell_size = 1;
+  for (int i = 0; i < ndim; i++) {
+    if (cell_size > INT64_MAX / cell_shape) {
+      free(shape);
+      free(chunkshape);
+      free(blockshape);
+      BLOSC_TRACE_ERROR("cell_shape %d too large for ndim %d", cell_shape, ndim);
+      return BLOSC2_ERROR_FAILURE;
+    }
+    cell_size *= cell_shape;
+  }
 
-  int32_t blocksize = (int32_t) typesize;
+  if (typesize <= 0) {
+    free(shape);
+    free(chunkshape);
+    free(blockshape);
+    BLOSC_TRACE_ERROR("Invalid typesize %d", typesize);
+    return BLOSC2_ERROR_FAILURE;
+  }
+  // See ndcell_backward(): validate the b2nd block geometry in 64-bit so a
+  // crafted blockshape cannot wrap a 32-bit product back onto `length` and make
+  // the index arithmetic below read past the `length`-sized input buffer.
+  int64_t blocksize = (int64_t) typesize;
   for (int i = 0; i < ndim; i++) {
+    if (blockshape[i] <= 0) {
+      free(shape);
+      free(chunkshape);
+      free(blockshape);
+      BLOSC_TRACE_ERROR("Invalid blockshape[%d] = %d", i, blockshape[i]);
+      return BLOSC2_ERROR_FAILURE;
+    }
     blocksize *= blockshape[i];
+    if (blocksize > (int64_t) length) {
+      free(shape);
+      free(chunkshape);
+      free(blockshape);
+      BLOSC_TRACE_ERROR("blockshape too large for block of %d bytes", length);
+      return BLOSC2_ERROR_FAILURE;
+    }
   }
 
-  if (length != blocksize) {
+  if ((int64_t) length != blocksize) {
     free(shape);
     free(chunkshape);
     free(blockshape);
-    BLOSC_TRACE_ERROR("Length not equal to blocksize %d %d \n", length, blocksize);
+    BLOSC_TRACE_ERROR("Length not equal to blocksize %d %lld \n", length, (long long) blocksize);
     return BLOSC2_ERROR_FAILURE;
   }
 
   uint8_t *ip = (uint8_t *) input;
   uint8_t *op = (uint8_t *) output;
   uint8_t *op_limit = op + length;
 
-  if (length < cell_size * typesize) {
+  // length is a multiple of typesize here (length == blocksize, checked above),
+  // so the division is exact; phrasing the check this way avoids overflowing the
+  // cell_size * typesize product.
+  if (cell_size > (int64_t) length / typesize) {
     free(shape);
     free(chunkshape);
     free(blockshape);
@@ -182,26 +234,82 @@ int ndcell_backward(const uint8_t *input, uint8_t *output, int32_t length, uint8
     return BLOSC2_ERROR_FAILURE;
   }
 
-  int8_t cell_shape = (int8_t) meta;
-  int cell_size = (int) pow(cell_shape, ndim);
+  // `meta` is attacker-controlled in a crafted frame.  Interpret it as unsigned
+  // and reject 0: an (int8_t) cast would turn 128..255 into negative values and
+  // 0 would later divide by zero in `(blockshape[i] + cell_shape - 1) / cell_shape`.
+  int cell_shape = (int) meta;
+  if (cell_shape <= 0) {
+    free(shape);
+    free(chunkshape);
+    free(blockshape);
+    BLOSC_TRACE_ERROR("Invalid cell_shape %d", cell_shape);
+    return BLOSC2_ERROR_FAILURE;
+  }
+  // See ndcell_forward(): compute cell_shape^ndim with checked 64-bit
+  // multiplication so an overflowing (int) pow() cannot bypass the
+  // "buffer smaller than cell" check below.
+  int64_t cell_size = 1;
+  for (int i = 0; i < ndim; i++) {
+    if (cell_size > INT64_MAX / cell_shape) {
+      free(shape);
+      free(chunkshape);
+      free(blockshape);
+      BLOSC_TRACE_ERROR("cell_shape %d too large for ndim %d", cell_shape, ndim);
+      return BLOSC2_ERROR_FAILURE;
+    }
+    cell_size *= cell_shape;
+  }
   int32_t typesize = schunk->typesize;
   uint8_t *ip = (uint8_t *) input;
   uint8_t *ip_limit = ip + length;
   uint8_t *op = (uint8_t *) output;
-  int32_t blocksize = (int32_t) typesize;
+  if (typesize <= 0) {
+    free(shape);
+    free(chunkshape);
+    free(blockshape);
+    BLOSC_TRACE_ERROR("Invalid typesize %d", typesize);
+    return BLOSC2_ERROR_FAILURE;
+  }
+  // The block geometry (blockshape) comes from the attacker-controlled "b2nd"
+  // metalayer and is NOT validated by b2nd_deserialize_meta().  Compute the
+  // block size in 64-bit and reject non-positive dimensions.  Otherwise a
+  // crafted blockshape whose 32-bit product wraps back onto `length` would pass
+  // the check below, while the scatter-write index arithmetic further down uses
+  // the true (huge) dimensions, writing past the `length`-sized output buffer
+  // (heap overflow).  Bailing as soon as the running product exceeds `length`
+  // also keeps the 64-bit multiply from overflowing (it stays
+  // <= length * INT32_MAX < INT64_MAX).
+  int64_t blocksize = (int64_t) typesize;
   for (int i = 0; i < ndim; i++) {
+    if (blockshape[i] <= 0) {
+      free(shape);
+      free(chunkshape);
+      free(blockshape);
+      BLOSC_TRACE_ERROR("Invalid blockshape[%d] = %d", i, blockshape[i]);
+      return BLOSC2_ERROR_FAILURE;
+    }
     blocksize *= blockshape[i];
+    if (blocksize > (int64_t) length) {
+      free(shape);
+      free(chunkshape);
+      free(blockshape);
+      BLOSC_TRACE_ERROR("blockshape too large for block of %d bytes", length);
+      return BLOSC2_ERROR_FAILURE;
+    }
   }
 
-  if (length != blocksize) {
+  if ((int64_t) length != blocksize) {
     free(shape);
     free(chunkshape);
     free(blockshape);
-    BLOSC_TRACE_ERROR("Length not equal to blocksize");
+    BLOSC_TRACE_ERROR("Length not equal to blocksize %d %lld", length, (long long) blocksize);
     return BLOSC2_ERROR_FAILURE;
   }
 
-  if (length < cell_size * typesize) {
+  // length is a multiple of typesize here (length == blocksize, checked above),
+  // so the division is exact; phrasing the check this way avoids overflowing the
+  // cell_size * typesize product.
+  if (cell_size > (int64_t) length / typesize) {
     free(shape);
     free(chunkshape);
     free(blockshape);
@@ -272,8 +380,8 @@ int ndcell_backward(const uint8_t *input, uint8_t *output, int32_t length, uint8
     free(shape);
     free(chunkshape);
     free(blockshape);
-    BLOSC_TRACE_ERROR("Output size is not compatible with embedded blockshape ind %d %d \n",
-                      ind, (blocksize / typesize));
+    BLOSC_TRACE_ERROR("Output size is not compatible with embedded blockshape ind %d %lld \n",
+                      ind, (long long) (blocksize / typesize));
     return BLOSC2_ERROR_FAILURE;
   }
 

plugins/filters/ndcell/test_ndcell_oversized_blockshape.c

@@ -0,0 +1,135 @@
+/*********************************************************************
+    Blosc - Blocked Shuffling and Compression Library
+
+    Copyright (c) 2021  Blosc Development Team <blosc@blosc.org>
+    https://blosc.org
+    License: BSD 3-Clause (see LICENSE.txt)
+
+    See LICENSE.txt for details about copyright and rights to use.
+
+    Regression test for the NDCELL filter hardening in ndcell.c.
+
+    ndcell_backward() takes the block geometry (blockshape) from the "b2nd"
+    metalayer, which is attacker-controlled in a crafted frame, while the real
+    output block buffer is sized from the chunk-header block size (`length`).
+    The old code computed   blocksize = typesize * prod(blockshape)   in 32-bit
+    arithmetic: a crafted blockshape whose product wraps back onto `length`
+    passed the `length == blocksize` check, after which the scatter-write
+    index arithmetic used the *true* (huge) dimensions and wrote past the
+    output buffer (heap overflow).
+
+    These cases exercise the rejection paths so the fix cannot regress silently:
+      - blockshape whose 32-bit product wraps back onto `length`
+      - a zero block dimension (must be invalid, not a size-0 shortcut)
+      - a negative block dimension
+**********************************************************************/
+
+#include "b2nd.h"
+#include "blosc2.h"
+#include "ndcell.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+
+/* ndcell_backward() is declared in ndcell.h (included above). */
+
+static blosc2_schunk *make_schunk(int8_t ndim, const int64_t *shape, const int32_t *chunkshape,
+                                  const int32_t *blockshape, int32_t typesize) {
+  blosc2_storage storage = BLOSC2_STORAGE_DEFAULTS;
+  blosc2_cparams cparams = BLOSC2_CPARAMS_DEFAULTS;
+  cparams.typesize = typesize;
+  storage.cparams = &cparams;
+  blosc2_schunk *schunk = blosc2_schunk_new(&storage);
+  if (schunk == NULL) {
+    return NULL;
+  }
+  uint8_t *smeta = NULL;
+  int smeta_len = b2nd_serialize_meta(ndim, shape, chunkshape, blockshape, "|u1", 0, &smeta);
+  if (smeta_len < 0) {
+    blosc2_schunk_free(schunk);
+    return NULL;
+  }
+  /* The metalayer MUST be attached, otherwise the decoder would fail for the
+     wrong reason ("b2nd layer not found") and the test would pass vacuously. */
+  int rc = blosc2_meta_add(schunk, "b2nd", smeta, smeta_len);
+  free(smeta);
+  if (rc < 0) {
+    blosc2_schunk_free(schunk);
+    return NULL;
+  }
+  return schunk;
+}
+
+/* ndcell_backward must reject this geometry (return < 0) without writing past
+   `length`.  Returns 0 on success (rejected), -1 on failure. */
+static int expect_rejected(const char *name, int32_t b0, int32_t b1,
+                           int32_t length, int32_t typesize, uint8_t cell_shape) {
+  int64_t shape[2]      = { b0, b1 };
+  int32_t chunkshape[2] = { b0, b1 };
+  int32_t blockshape[2] = { b0, b1 };
+  blosc2_schunk *schunk = make_schunk(2, shape, chunkshape, blockshape, typesize);
+  if (schunk == NULL) {
+    printf("  %-30s: could not build schunk -- FAIL\n", name);
+    return -1;
+  }
+
+  blosc2_dparams dparams = BLOSC2_DPARAMS_DEFAULTS;
+  dparams.schunk = schunk;
+
+  size_t out_len = (size_t) (length > 0 ? length : 1);
+  /* Place a canary region immediately after the output buffer.  A rejection
+     must happen *before* any scatter-write, so even a partial overflow that
+     precedes the error return is caught here -- not just by the return code. */
+  const size_t canary_len = 64;
+  const uint8_t canary_byte = 0xAC;
+  uint8_t *input       = calloc(1, out_len);
+  uint8_t *output_full = malloc(out_len + canary_len);
+  uint8_t *output      = output_full;
+  memset(output_full + out_len, canary_byte, canary_len);
+
+  int rc = ndcell_backward(input, output, length, cell_shape, &dparams, 0);
+
+  int canary_ok = 1;
+  for (size_t i = 0; i < canary_len; i++) {
+    if (output_full[out_len + i] != canary_byte) {
+      canary_ok = 0;
+      break;
+    }
+  }
+
+  free(input);
+  free(output_full);
+  blosc2_schunk_free(schunk);
+
+  if (!canary_ok) {
+    printf("  %-30s: wrote past output buffer (canary clobbered) -- FAIL\n", name);
+    return -1;
+  }
+  if (rc >= 0) {
+    printf("  %-30s: NOT rejected (rc=%d) -- FAIL\n", name, rc);
+    return -1;
+  }
+  printf("  %-30s: rejected (rc=%d) -- OK\n", name, rc);
+  return 0;
+}
+
+int main(void) {
+  int result = 0;
+  blosc2_init();   // mandatory for initializing the plugin mechanism
+  printf("NDCELL decompress hardening regression tests:\n");
+
+  /* 1*65536*65537 = 0x100010000, low 32 bits = 0x10000 = 65536 == length */
+  result |= expect_rejected("32-bit wrap onto length", 65536, 65537, 65536, 1, 2);
+  /* a zero block dimension must be invalid input */
+  result |= expect_rejected("zero block dimension", 0, 64, 256, 1, 2);
+  /* a negative block dimension must be invalid input */
+  result |= expect_rejected("negative block dimension", -1, 64, 256, 1, 2);
+
+  if (result == 0) {
+    printf("All NDCELL hardening checks passed.\n");
+  }
+  blosc2_destroy();
+  return result;
+}