[FIX] Prevent buffer overrun when destination is small and we are mem…

…cpying
Blosc · Jan 26, 2018 · ac47cac · ac47cac
1 parent 3f42e0e
commit ac47cac
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 18 deletions.
diff --git a/blosc/blosc.c b/blosc/blosc.c
@@ -1174,20 +1174,25 @@ int blosc_compress_context(struct blosc_context* context)
 {
   int32_t ntbytes = 0;
 
-    /* Do the actual compression */
+  if ((*(context->header_flags) & BLOSC_MEMCPYED) &&
+      (context->sourcesize + BLOSC_MAX_OVERHEAD > context->destsize)) {
+    return 0;   /* data cannot be copied without overrun destination */
+  }
+
+  /* Do the actual compression */
+  ntbytes = do_job(context);
+  if (ntbytes < 0) {
+    return -1;
+  }
+  if ((ntbytes == 0) && (context->sourcesize + BLOSC_MAX_OVERHEAD <= context->destsize)) {
+    /* Last chance for fitting `src` buffer in `dest`.  Update flags and force a copy. */
+    *(context->header_flags) |= BLOSC_MEMCPYED;
+    context->num_output_bytes = BLOSC_MAX_OVERHEAD;  /* reset the output bytes in previous step */
     ntbytes = do_job(context);
     if (ntbytes < 0) {
       return -1;
     }
-    if ((ntbytes == 0) && (context->sourcesize+BLOSC_MAX_OVERHEAD <= context->destsize)) {
-      /* Last chance for fitting `src` buffer in `dest`.  Update flags and force a copy. */
-      *(context->header_flags) |= BLOSC_MEMCPYED;
-      context->num_output_bytes = BLOSC_MAX_OVERHEAD;  /* reset the output bytes in previous step */
-      ntbytes = do_job(context);
-      if (ntbytes < 0) {
-        return -1;
-      }
-    }
+  }
 
   /* Set the number of compressed bytes in header */
   _sw32(context->dest + 12, ntbytes);

diff --git a/tests/test_maxout.c b/tests/test_maxout.c
@@ -15,7 +15,7 @@ int tests_run = 0;
 
 /* Global vars */
 void *src, *srccpy, *dest, *dest2;
-size_t nbytes, cbytes;
+int nbytes, cbytes;
 int clevel = 1;
 int doshuffle = 0;
 size_t typesize = 4;
@@ -26,8 +26,18 @@ size_t size = 1000;             /* must be divisible by 4 */
 static char *test_maxout_less() {
 
   /* Get a compressed buffer */
-  cbytes = blosc_compress(clevel, doshuffle, typesize, size, src,
-                          dest, size+15);
+  cbytes = blosc_compress(clevel, doshuffle, typesize, size, src, dest, size+15);
+  mu_assert("ERROR: cbytes is not 0", cbytes == 0);
+
+  return 0;
+}
+
+
+/* Check maxout with maxout < size (memcpy version) */
+static char *test_maxout_less_memcpy() {
+
+  /* Get a compressed buffer */
+  cbytes = blosc_compress(0, doshuffle, typesize, size, src, dest, size+15);
   mu_assert("ERROR: cbytes is not 0", cbytes == 0);
 
   return 0;
@@ -38,8 +48,22 @@ static char *test_maxout_less() {
 static char *test_maxout_equal() {
 
   /* Get a compressed buffer */
-  cbytes = blosc_compress(clevel, doshuffle, typesize, size, src,
-                          dest, size+16);
+  cbytes = blosc_compress(clevel, doshuffle, typesize, size, src, dest, size+16);
+  mu_assert("ERROR: cbytes is not correct", cbytes == size+16);
+
+  /* Decompress the buffer */
+  nbytes = blosc_decompress(dest, dest2, size);
+  mu_assert("ERROR: nbytes incorrect(1)", nbytes == size);
+
+  return 0;
+}
+
+
+/* Check maxout with maxout == size (memcpy version) */
+static char *test_maxout_equal_memcpy() {
+
+  /* Get a compressed buffer */
+  cbytes = blosc_compress(0, doshuffle, typesize, size, src, dest, size+16);
   mu_assert("ERROR: cbytes is not correct", cbytes == size+16);
 
   /* Decompress the buffer */
@@ -53,9 +77,22 @@ static char *test_maxout_equal() {
 /* Check maxout with maxout > size */
 static char *test_maxout_great() {
   /* Get a compressed buffer */
-  cbytes = blosc_compress(clevel, doshuffle, typesize, size, src,
-                          dest, size+17);
-  mu_assert("ERROR: cbytes is not 0", cbytes == size+16);
+  cbytes = blosc_compress(clevel, doshuffle, typesize, size, src, dest, size+17);
+  mu_assert("ERROR: cbytes is not correct", cbytes == size+16);
+
+  /* Decompress the buffer */
+  nbytes = blosc_decompress(dest, dest2, size);
+  mu_assert("ERROR: nbytes incorrect(1)", nbytes == size);
+
+  return 0;
+}
+
+
+/* Check maxout with maxout > size (memcpy version) */
+static char *test_maxout_great_memcpy() {
+  /* Get a compressed buffer */
+  cbytes = blosc_compress(0, doshuffle, typesize, size, src, dest, size+17);
+  mu_assert("ERROR: cbytes is not correct", cbytes == size+16);
 
   /* Decompress the buffer */
   nbytes = blosc_decompress(dest, dest2, size);
@@ -67,8 +104,11 @@ static char *test_maxout_great() {
 
 static char *all_tests() {
   mu_run_test(test_maxout_less);
+  mu_run_test(test_maxout_less_memcpy);
   mu_run_test(test_maxout_equal);
+  mu_run_test(test_maxout_equal_memcpy);
   mu_run_test(test_maxout_great);
+  mu_run_test(test_maxout_great_memcpy);
 
   return 0;
 }