perform size checks on memcpy/memmove/memset

- memset is disabled for now as it causes hangs
- underlying functions were copied from isoalloc, licensed Apache-2.0
	- credit Chris Rohlf for memcpy/memset
	- credit David Carlier for memmove
- use the fast path as some programs crash otherwise

Signed-off-by: Tavi <tavi@divested.dev>

Android.bp

@@ -28,6 +28,7 @@ common_cflags = [
     "-DN_ARENA=1",
     "-DCONFIG_STATS=true",
     "-DCONFIG_SELF_INIT=false",
+    "-DCONFIG_BLOCK_OPS_CHECK_SIZE=false",
 ]
 
 cc_defaults {

CREDITS

@@ -23,6 +23,12 @@ h_malloc.c open-addressed hash table (regions_grow, regions_insert, regions_find
     ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
     OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
+h_malloc.c block operations (h_memcpy_real, h_memmove_real, h_memset_real):
+
+    Copyright (C) 2022, 2023 struct <chris.rohlf@gmail.com>
+    Copyright (C) 2023 David Carlier <devnexen@gmail.com>
+    Apache-2.0
+
 libdivide:
 
     Copyright (C) 2010 - 2019 ridiculous_fish, <libdivide@ridiculousfish.com>

Makefile

@@ -89,6 +89,10 @@ ifeq (,$(filter $(CONFIG_SELF_INIT),true false))
     $(error CONFIG_SELF_INIT must be true or false)
 endif
 
+ifeq (,$(filter $(CONFIG_BLOCK_OPS_CHECK_SIZE),true false))
+    $(error CONFIG_BLOCK_OPS_CHECK_SIZE must be true or false)
+endif
+
 CPPFLAGS += \
     -DCONFIG_SEAL_METADATA=$(CONFIG_SEAL_METADATA) \
     -DZERO_ON_FREE=$(CONFIG_ZERO_ON_FREE) \
@@ -108,7 +112,8 @@ CPPFLAGS += \
     -DCONFIG_CLASS_REGION_SIZE=$(CONFIG_CLASS_REGION_SIZE) \
     -DN_ARENA=$(CONFIG_N_ARENA) \
     -DCONFIG_STATS=$(CONFIG_STATS) \
-    -DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT)
+    -DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT) \
+    -DCONFIG_BLOCK_OPS_CHECK_SIZE=$(CONFIG_BLOCK_OPS_CHECK_SIZE)
 
 $(OUT)/libhardened_malloc$(SUFFIX).so: $(OBJECTS) | $(OUT)
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared $^ $(LDLIBS) -o $@

README.md

@@ -276,6 +276,10 @@ The following boolean configuration options are available:
   hardware, which may become drastically lower in the future. Whether or not
   this feature is enabled, the metadata is all contained within an isolated
   memory region with high entropy random guard regions around it.
+* `CONFIG_BLOCK_OPS_CHECK_SIZE`: `true` or `false` (default) to ensure length
+  parameter of the memcpy/memmove/memset block operations are within
+  approximate bounds to minimize buffer overflows. Note, memset override is
+  currently disabled due to improper behavior.
 
 The following integer configuration options are available:
 

config/default.mk

@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
 CONFIG_N_ARENA := 4
 CONFIG_STATS := false
 CONFIG_SELF_INIT := true
+CONFIG_BLOCK_OPS_CHECK_SIZE := false

config/light.mk

@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
 CONFIG_N_ARENA := 4
 CONFIG_STATS := false
 CONFIG_SELF_INIT := true
+CONFIG_BLOCK_OPS_CHECK_SIZE := false

h_malloc.c

@@ -1874,6 +1874,80 @@ EXPORT size_t h_malloc_object_size_fast(const void *p) {
     return SIZE_MAX;
 }
 
+#if CONFIG_BLOCK_OPS_CHECK_SIZE
+inline void *h_memcpy_real(void *dst, const void *src, size_t len) {
+    char *p_dst = (char *)dst;
+    char const *p_src = (char const *)src;
+
+    while(len--) {
+        *p_dst++ = *p_src++;
+    }
+
+    return dst;
+}
+
+EXPORT void *h_memcpy(void *dst, const void *src, size_t len) {
+    if (len > malloc_object_size_fast(src)) {
+        fatal_error("memcpy read overflow");
+    }
+    if (len > malloc_object_size_fast(dst)) {
+        fatal_error("memcpy buffer overflow");
+    }
+
+    return h_memcpy_real(dst, src, len);
+}
+
+inline void *h_memmove_real(void *dst, const void *src, size_t len) {
+    char *p_dst = (char *)dst;
+    char const *p_src = (char const *)src;
+
+    if(dst == src) {
+        return dst;
+    }
+
+    if(p_src < p_dst) {
+        p_dst += len;
+        p_src += len;
+        while(len--) {
+            *--p_dst = *--p_src;
+        }
+    } else {
+        dst = h_memcpy(dst, src, len);
+    }
+
+    return dst;
+}
+
+EXPORT void *h_memmove(void *dst, const void *src, size_t len) {
+    if (len > malloc_object_size_fast(src)) {
+        fatal_error("memmove read overflow");
+    }
+    if (len > malloc_object_size_fast(dst)) {
+        fatal_error("memmove buffer overflow");
+    }
+
+    return h_memmove_real(dst, src, len);
+}
+
+inline void *h_memset_real(void *dst, int value, size_t len) {
+    char *p_dst = (char *)dst;
+
+    while(len--) {
+        *p_dst++ = value;
+    }
+
+    return dst;
+}
+
+EXPORT void *h_memset(void *dst, int value, size_t len) {
+    if (len > malloc_object_size_fast(dst)) {
+        fatal_error("memset buffer overflow");
+    }
+
+    return h_memset_real(dst, value, len);
+}
+#endif
+
 EXPORT int h_mallopt(UNUSED int param, UNUSED int value) {
 #ifdef __ANDROID__
     if (param == M_PURGE) {

include/h_malloc.h

@@ -15,6 +15,11 @@ extern "C" {
 #define h_realloc realloc
 #define h_aligned_alloc aligned_alloc
 #define h_free free
+#if CONFIG_BLOCK_OPS_CHECK_SIZE
+#define h_memcpy memcpy
+#define h_memmove memmove
+//#define h_memset memset
+#endif
 
 #define h_posix_memalign posix_memalign
 
@@ -54,6 +59,14 @@ __attribute__((alloc_size(2))) void *h_realloc(void *ptr, size_t size);
 __attribute__((malloc)) __attribute__((alloc_size(2))) __attribute__((alloc_align(1)))
 void *h_aligned_alloc(size_t alignment, size_t size);
 void h_free(void *ptr);
+#if CONFIG_BLOCK_OPS_CHECK_SIZE
+void *h_memcpy_real(void *dst, const void *src, size_t len);
+void *h_memcpy(void *dst, const void *src, size_t len);
+void *h_memmove_real(void *dst, const void *src, size_t len);
+void *h_memmove(void *dst, const void *src, size_t len);
+void *h_memset_real(void *dst, int value, size_t len);
+void *h_memset(void *dst, int value, size_t len);
+#endif
 
 // POSIX
 int h_posix_memalign(void **memptr, size_t alignment, size_t size);

test/.gitignore

@@ -41,4 +41,15 @@ overflow_small_8_byte
 uninitialized_read_large
 uninitialized_read_small
 realloc_init
+memcpy_buffer_overflow
+memcpy_read_overflow
+memcpy_valid_same
+memcpy_valid_mismatched
+memmove_buffer_overflow
+memmove_read_overflow
+memmove_valid_same
+memmove_valid_mismatched
+memset_buffer_overflow
+memset_valid_same
+memset_valid_mismatched
 __pycache__/

test/Makefile

@@ -67,7 +67,18 @@ EXECUTABLES := \
     invalid_malloc_object_size_small \
     invalid_malloc_object_size_small_quarantine \
     impossibly_large_malloc \
-    realloc_init
+    realloc_init \
+    memcpy_buffer_overflow \
+    memcpy_read_overflow \
+    memcpy_valid_same \
+    memcpy_valid_mismatched \
+    memmove_buffer_overflow \
+    memmove_read_overflow \
+    memmove_valid_same \
+    memmove_valid_mismatched \
+    memset_buffer_overflow \
+    memset_valid_same \
+    memset_valid_mismatched
 
 all: $(EXECUTABLES)
 

test/memcpy_buffer_overflow.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(16);
+    char *secondbuffer = malloc(32);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 16);
+    memcpy(firstbuffer, secondbuffer, 32);
+    return 1;
+}

test/memcpy_read_overflow.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(32);
+    char *secondbuffer = malloc(16);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 16);
+    memcpy(firstbuffer, secondbuffer, 32);
+    return 1;
+}

test/memcpy_valid_mismatched.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(16);
+    char *secondbuffer = malloc(8);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 8);
+    memcpy(firstbuffer, secondbuffer, 8);
+    return 0;
+}

test/memcpy_valid_same.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(16);
+    char *secondbuffer = malloc(16);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 16);
+    memcpy(firstbuffer, secondbuffer, 16);
+    return 0;
+}

test/memmove_buffer_overflow.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(16);
+    char *secondbuffer = malloc(32);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 16);
+    memmove(firstbuffer, secondbuffer, 32);
+    return 1;
+}

test/memmove_read_overflow.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(32);
+    char *secondbuffer = malloc(16);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 16);
+    memmove(firstbuffer, secondbuffer, 32);
+    return 1;
+}

test/memmove_valid_mismatched.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(16);
+    char *secondbuffer = malloc(8);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 8);
+    memmove(firstbuffer, secondbuffer, 8);
+    return 0;
+}

test/memmove_valid_same.c

@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *firstbuffer = malloc(16);
+    char *secondbuffer = malloc(16);
+    if (!firstbuffer && !secondbuffer) {
+        return 1;
+    }
+    memset(secondbuffer, 'a', 16);
+    memmove(firstbuffer, secondbuffer, 16);
+    return 0;
+}

test/memset_buffer_overflow.c

@@ -0,0 +1,13 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *buffer = malloc(16);
+    if (!buffer) {
+        return 1;
+    }
+    memset(buffer, 'a', 32);
+    return 1;
+}

test/memset_valid_mismatched.c

@@ -0,0 +1,13 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *buffer = malloc(16);
+    if (!buffer) {
+        return 1;
+    }
+    memset(buffer, 'a', 8);
+    return 0;
+}

test/memset_valid_same.c

@@ -0,0 +1,13 @@
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+
+OPTNONE int main(void) {
+    char *buffer = malloc(16);
+    if (!buffer) {
+        return 1;
+    }
+    memset(buffer, 'a', 16);
+    return 0;
+}

test/test_smc.py

@@ -238,5 +238,70 @@ class TestSimpleMemoryCorruption(unittest.TestCase):
             "realloc_init")
         self.assertEqual(returncode, 0)
 
+    #def test_memcpy_buffer_overflow(self):
+    #    _stdout, stderr, returncode = self.run_test(
+    #        "memcpy_buffer_overflow")
+    #    self.assertEqual(returncode, -6)
+    #    self.assertEqual(stderr.decode(
+    #        "utf-8"), "fatal allocator error: memcpy buffer overflow\n")
+
+    #def test_memcpy_read_overflow(self):
+    #    _stdout, stderr, returncode = self.run_test(
+    #        "memcpy_read_overflow")
+    #    self.assertEqual(returncode, -6)
+    #    self.assertEqual(stderr.decode(
+    #        "utf-8"), "fatal allocator error: memcpy read overflow\n")
+
+    def test_memcpy_valid_same(self):
+        _stdout, _stderr, returncode = self.run_test(
+            "memcpy_valid_same")
+        self.assertEqual(returncode, 0)
+
+    def test_memcpy_valid_mismatched(self):
+        _stdout, _stderr, returncode = self.run_test(
+            "memcpy_valid_mismatched")
+        self.assertEqual(returncode, 0)
+
+    #def test_memmove_buffer_overflow(self):
+    #    _stdout, stderr, returncode = self.run_test(
+    #        "memmove_buffer_overflow")
+    #    self.assertEqual(returncode, -6)
+    #    self.assertEqual(stderr.decode(
+    #        "utf-8"), "fatal allocator error: memmove buffer overflow\n")
+
+    #def test_memmove_read_overflow(self):
+    #    _stdout, stderr, returncode = self.run_test(
+    #        "memmove_read_overflow")
+    #    self.assertEqual(returncode, -6)
+    #    self.assertEqual(stderr.decode(
+    #        "utf-8"), "fatal allocator error: memmove read overflow\n")
+
+    def test_memmove_valid_same(self):
+        _stdout, _stderr, returncode = self.run_test(
+            "memmove_valid_same")
+        self.assertEqual(returncode, 0)
+
+    def test_memmove_valid_mismatched(self):
+        _stdout, _stderr, returncode = self.run_test(
+            "memmove_valid_mismatched")
+        self.assertEqual(returncode, 0)
+
+    #def test_memset_buffer_overflow(self):
+    #    _stdout, stderr, returncode = self.run_test(
+    #        "memset_buffer_overflow")
+    #    self.assertEqual(returncode, -6)
+    #    self.assertEqual(stderr.decode(
+    #        "utf-8"), "fatal allocator error: memset buffer overflow\n")
+
+    def test_memset_valid_same(self):
+        _stdout, _stderr, returncode = self.run_test(
+            "memset_valid_same")
+        self.assertEqual(returncode, 0)
+
+    def test_memset_valid_mismatched(self):
+        _stdout, _stderr, returncode = self.run_test(
+            "memset_valid_mismatched")
+        self.assertEqual(returncode, 0)
+
 if __name__ == '__main__':
     unittest.main()