Merge pull request #107 from struct/new_libc_hook

add guarded memcpy with libc hook and security checks, update docs

Author: struct

Makefile

@@ -92,7 +92,6 @@ HUGE_PAGES = -DHUGE_PAGES=1
 ## README for more detailed information. This is Linux only
 ## and has negative performance implications
 CPU_PIN = -DCPU_PIN=0
-
 SCHED_GETCPU =
 
 ## Enable the allocation sanity feature. This works a lot
@@ -104,6 +103,11 @@ SCHED_GETCPU =
 ## much of a performance penalty
 ALLOC_SANITY = -DALLOC_SANITY=0
 
+## Enable hooking of memcpy to detect out of bounds r/w
+## operations on chunks allocated with IsoAlloc. Does
+## not require ALLOC_SANITY is enabled
+MEMCPY_SANITY = -DMEMCPY_SANITY=0
+
 ## Enable the userfaultfd based uninitialized read detection
 ## feature. This samples calls to malloc, and allocates raw
 ## pages of memory with mmap which are registered with the
@@ -192,9 +196,10 @@ else
 BUILD_ERROR_FLAGS := $(BUILD_ERROR_FLAGS) -Wno-attributes -Wno-unused-variable
 endif
 CFLAGS = $(COMMON_CFLAGS) $(SECURITY_FLAGS) $(BUILD_ERROR_FLAGS) $(HOOKS) $(HEAP_PROFILER) -fvisibility=hidden \
-	-std=c11 $(SANITIZER_SUPPORT) $(ALLOC_SANITY) $(UNINIT_READ_SANITY) $(CPU_PIN) $(SCHED_GETCPU) $(EXPERIMENTAL) $(UAF_PTR_PAGE) \
-	$(VERIFY_BIT_SLOT_CACHE) $(NAMED_MAPPINGS) $(ABORT_ON_NULL) $(NO_ZERO_ALLOCATIONS) $(ABORT_NO_ENTROPY) \
-	$(ISO_DTOR_CLEANUP) $(SHUFFLE_BIT_SLOT_CACHE) $(USE_SPINLOCK) $(HUGE_PAGES) $(USE_MLOCK) $(MEMORY_TAGGING)
+	-std=c11 $(SANITIZER_SUPPORT) $(ALLOC_SANITY) $(MEMCPY_SANITY) $(UNINIT_READ_SANITY) $(CPU_PIN) $(SCHED_GETCPU) \
+	$(EXPERIMENTAL) $(UAF_PTR_PAGE) $(VERIFY_BIT_SLOT_CACHE) $(NAMED_MAPPINGS) $(ABORT_ON_NULL) $(NO_ZERO_ALLOCATIONS) \
+	$(ABORT_NO_ENTROPY) $(ISO_DTOR_CLEANUP) $(SHUFFLE_BIT_SLOT_CACHE) $(USE_SPINLOCK) $(HUGE_PAGES) $(USE_MLOCK) \
+	$(MEMORY_TAGGING)
 CXXFLAGS = $(COMMON_CFLAGS) -DCPP_SUPPORT=1 -std=c++17 $(SANITIZER_SUPPORT) $(HOOKS)
 EXE_CFLAGS = -fPIE
 GDB_FLAGS = -g -ggdb3 -fno-omit-frame-pointer

README.md

@@ -85,6 +85,7 @@ When enabled, the `CPU_PIN` feature will restrict allocations from a given zone
 * When destroying private zones if `NEVER_REUSE_ZONES` is enabled IsoAlloc won't attempt to repurpose the zone
 * Zones are retired and replaced after they've allocated and freed a specific number of chunks. This is calculated as `ZONE_ALLOC_RETIRE * max_chunk_count_for_zone`.
 * When `MEMORY_TAGGING` is enabled IsoAlloc will create a 1 byte tag for each chunk in private zones. See the [MEMORY_TAGGING.md](MEMORY_TAGGING.md) documentation, or [this test](tests/tagged_ptr_test.cpp) for an example of how to use it.
+* When `MEMCPY_SANITY` is enabled the allocator will hook all calls to `memcpy` and check for out of bounds r/w operations when either src or dst points to a chunk allocated by IsoAlloc
 
 ## Building
 

SECURITY_COMPARISON.MD

@@ -37,7 +37,7 @@ Heap allocators hide incredible complexity behind `malloc` and `free`. They must
 |Zero Size Allocation Special Handling|:heavy_check_mark:|:x: |:grey_question:   |:grey_question:  |:x:               |:x:               |:x:               |:heavy_check_mark:|:x:               |:x:
 |Read-only global structure|:heavy_minus_sign:|:x:            |:x:               |:x:              |:x:               |:x:               |:x:               |:heavy_check_mark:|:x:               |:x:
 |SW Memory Tagging      |:heavy_minus_sign:|:x:               |:x:               |:x:              |:x:               |:x:               |:x:               |:x:               |:heavy_check_mark:|:x:
-|Guarded Memcpy         |:x:               |:x:               |:x:               |:x:              |:x:               |:x:               |:x:               |:x:               |:x:               |:heavy_check_mark:
+|Guarded Memcpy         |:heavy_check_mark:               |:x:               |:x:               |:x:              |:x:               |:x:               |:x:               |:x:               |:x:               |:heavy_check_mark:
 
 **Lexicon**
 

include/iso_alloc_sanity.h

@@ -11,6 +11,7 @@
 #pragma message "IsoAlloc is untested and unsupported on 32 bit platforms"
 #endif
 
+#if ALLOC_SANITY
 #if UNINIT_READ_SANITY
 #include <fcntl.h>
 #include <linux/userfaultfd.h>
@@ -79,3 +80,9 @@ INTERNAL_HIDDEN void *_iso_alloc_sample(const size_t size);
 INTERNAL_HIDDEN int32_t _iso_alloc_free_sane_sample(void *p);
 INTERNAL_HIDDEN int32_t _remove_from_sane_trace(void *p);
 INTERNAL_HIDDEN _sane_allocation_t *_get_sane_alloc(void *p);
+#endif
+
+#if MEMCPY_SANITY
+INTERNAL_HIDDEN void *__iso_memcpy(void *restrict dest, const void *restrict src, size_t n);
+INTERNAL_HIDDEN void *_iso_alloc_memcpy(void *restrict dest, const void *restrict src, size_t n);
+#endif

src/iso_alloc.c

@@ -1186,6 +1186,9 @@ INTERNAL_HIDDEN iso_alloc_zone_t *search_chunk_lookup_table(const void *restrict
         LOG_AND_ABORT("Pointer to zone lookup table corrupted at position %zu", ADDR_TO_CHUNK_TABLE(p));
     }
 
+    /* Its possible that zone_index is 0 and this is
+     * actually a cache miss. In this case we return
+     * the first zone and let the caller figure it out */
     return &_root->zones[zone_index];
 }
 

src/iso_alloc_interfaces.c

@@ -4,6 +4,10 @@
 #include "iso_alloc.h"
 #include "iso_alloc_internal.h"
 
+#if MEMCPY_SANITY
+#include "iso_alloc_sanity.h"
+#endif
+
 EXTERNAL_API NO_DISCARD MALLOC_ATTR ALLOC_SIZE ASSUME_ALIGNED void *iso_alloc(size_t size) {
     return _iso_alloc(NULL, size);
 }
@@ -57,7 +61,11 @@ EXTERNAL_API NO_DISCARD REALLOC_SIZE ASSUME_ALIGNED void *iso_realloc(void *p, s
     }
 
     if(p != NULL) {
+#if MEMCPY_SANITY
+        __iso_memcpy(r, p, size);
+#else
         __builtin_memcpy(r, p, size);
+#endif
     }
 
 #if PERM_FREE_REALLOC
@@ -100,7 +108,11 @@ EXTERNAL_API NO_DISCARD ASSUME_ALIGNED char *iso_strdup_from_zone(iso_alloc_zone
         return NULL;
     }
 
+#if MEMCPY_SANITY
+    __iso_memcpy(p, str, size);
+#else
     __builtin_memcpy(p, str, size);
+#endif
     return p;
 }
 
@@ -126,10 +138,18 @@ EXTERNAL_API NO_DISCARD ASSUME_ALIGNED char *iso_strndup_from_zone(iso_alloc_zon
     }
 
     if(s_size > n) {
+#if MEMCPY_SANITY
+        __iso_memcpy(p, str, n);
+#else
         __builtin_memcpy(p, str, n);
+#endif
         p[n - 1] = '\0';
     } else {
+#if MEMCPY_SANITY
+        __iso_memcpy(p, str, s_size);
+#else
         __builtin_memcpy(p, str, s_size);
+#endif
     }
 
     return p;

src/iso_alloc_profiler.c

@@ -2,6 +2,11 @@
  * Copyright 2022 - chris.rohlf@gmail.com */
 
 #include "iso_alloc_internal.h"
+
+#if MEMCPY_SANITY
+#include "iso_alloc_sanity.h"
+#endif
+
 #include <dlfcn.h>
 
 INTERNAL_HIDDEN uint64_t _iso_alloc_detect_leaks_in_zone(iso_alloc_zone_t *zone) {
@@ -219,15 +224,23 @@ INTERNAL_HIDDEN uint64_t __iso_alloc_big_zone_mem_usage() {
  * be used to interpret allocation patterns */
 INTERNAL_HIDDEN size_t _iso_get_alloc_traces(iso_alloc_traces_t *traces_out) {
     LOCK_ROOT();
-    memcpy(traces_out, _alloc_bts, sizeof(iso_alloc_traces_t));
+#if MEMCPY_SANITY
+    __iso_memcpy(traces_out, _alloc_bts, sizeof(iso_alloc_traces_t));
+#else
+    __builtin_memcpy(traces_out, _alloc_bts, sizeof(iso_alloc_traces_t));
+#endif
     size_t sz = _alloc_bts_count;
     UNLOCK_ROOT();
     return sz;
 }
 
 INTERNAL_HIDDEN size_t _iso_get_free_traces(iso_free_traces_t *traces_out) {
     LOCK_ROOT();
-    memcpy(traces_out, _free_bts, sizeof(iso_free_traces_t));
+#if MEMCPY_SANITY
+    __iso_memcpy(traces_out, _free_bts, sizeof(iso_free_traces_t));
+#else
+    __builtin_memcpy(traces_out, _free_bts, sizeof(iso_free_traces_t));
+#endif
     size_t sz = _free_bts_count;
     UNLOCK_ROOT();
     return sz;

src/iso_alloc_sanity.c

@@ -274,5 +274,43 @@ INTERNAL_HIDDEN void *_iso_alloc_sample(const size_t size) {
     UNLOCK_SANITY_CACHE();
     return p;
 }
+#endif
+
+#if MEMCPY_SANITY
+INTERNAL_HIDDEN void *__iso_memcpy(void *restrict dest, const void *restrict src, size_t n) {
+    char *p_dest = (char *) dest;
+    char const *p_src = (char const *) src;
+
+    while(n--) {
+        *p_dest++ = *p_src++;
+    }
 
+    return dest;
+}
+
+INTERNAL_HIDDEN void *_iso_alloc_memcpy(void *restrict dest, const void *restrict src, size_t n) {
+    if(n > SMALLEST_CHUNK_SZ) {
+        /* We don't want to add too much overhead here so we only
+         * check the chunk-to-zone cache for zone data and we don't
+         * need to lock the root for that. Its possible for a cache
+         * miss to mean a security check doesn't happen here but
+         * this feature is more for catching bugs than it is for
+         * mitigating them */
+        iso_alloc_zone_t *zone = search_chunk_lookup_table(dest);
+        void *user_pages_start = UNMASK_USER_PTR(zone);
+
+        if(user_pages_start <= dest && (user_pages_start + ZONE_USER_SIZE) > dest && n > zone->chunk_size) {
+            LOG_AND_ABORT("Detected an out of bounds write memcpy: dest=0x%p (%d bytes) src=0x%p size=%d", dest, zone->chunk_size, src, n);
+        }
+
+        zone = search_chunk_lookup_table(src);
+        user_pages_start = UNMASK_USER_PTR(zone);
+
+        if(user_pages_start <= src && (user_pages_start + ZONE_USER_SIZE) > src && n > zone->chunk_size) {
+            LOG_AND_ABORT("Detected an out of bounds read memcpy: dest=0x%p src=0x%p (%d bytes) size=%d", dest, src, zone->chunk_size, n);
+        }
+    }
+
+    return __iso_memcpy(dest, src, n);
+}
 #endif

src/iso_alloc_util.c

@@ -6,16 +6,14 @@
 #if CPU_PIN
 /* sched_getcpu's performance depends on the
  * architecture/kernel version, so we lower
- * the cost of feature's abstraction here.
- */
+ * the cost of feature's abstraction here. */
 INTERNAL_HIDDEN INLINE int _iso_getcpu(void) {
 #if defined(SCHED_GETCPU)
     return sched_getcpu();
 #elif defined(__x86_64__)
     /* rdtscp is not always available and is pretty slow
      * we instead load from the global descriptor table
-     * then "mov" it to a.
-     */
+     * then "mov" it to 'a' */
     unsigned int a;
     const unsigned int cpunodesegment = 15 * 8 + 3;
     __asm__ volatile("lsl %1, %0"
@@ -26,11 +24,11 @@ INTERNAL_HIDDEN INLINE int _iso_getcpu(void) {
 #if __APPLE__
     /* unlike other operating systems, the tpidr_el0 register on macOs
      * is unused data stored for the current thread is instead fetchable
-     * from "tpidrro_el0".
-     */
+     * from "tpidrro_el0". */
     uintptr_t a;
-    __asm__ volatile("mrs %x0, tpidrro_el0" : "=r"(a) :: "memory");
-    return (int)((a & 0x8) - 1);
+    __asm__ volatile("mrs %x0, tpidrro_el0"
+                     : "=r"(a)::"memory");
+    return (int) ((a & 0x8) - 1);
 #else
     /* TODO most likely different register/making on other platforms */
     return -1;

src/libc_hook.c

@@ -0,0 +1,13 @@
+/* libc_hook.c - Provides low level hooks for libc functions
+ * Copyright 2022 - chris.rohlf@gmail.com */
+
+#include "iso_alloc_internal.h"
+#include "iso_alloc_sanity.h"
+
+#if MEMCPY_SANITY
+
+EXTERNAL_API void *memcpy(void *restrict dest, const void *restrict src, size_t n) {
+    return _iso_alloc_memcpy(dest, src, n);
+}
+
+#endif