add 0 byte to canary to prevent spurious read overflow to read the canary (issue #951, pr #953)

include/mimalloc/internal.h

@@ -667,6 +667,16 @@ static inline mi_encoded_t mi_ptr_encode(const void* null, const void* p, const
   return mi_rotl(x ^ keys[1], keys[0]) + keys[0];
 }
 
+static inline uint32_t mi_ptr_encode_canary(const void* null, const void* p, const uintptr_t* keys) {
+  const uint32_t x = (uint32_t)(mi_ptr_encode(null,p,keys));
+  // make the lowest byte 0 to prevent spurious read overflows which could be a security issue (issue #951)
+  #ifdef MI_BIG_ENDIAN
+  return (x & 0x00FFFFFF);
+  #else
+  return (x & 0xFFFFFF00);
+  #endif
+}
+
 static inline mi_block_t* mi_block_nextx( const void* null, const mi_block_t* block, const uintptr_t* keys ) {
   mi_track_mem_defined(block,sizeof(mi_block_t));
   mi_block_t* next;

src/alloc.c

@@ -99,7 +99,7 @@ extern inline void* _mi_page_malloc_zero(mi_heap_t* heap, mi_page_t* page, size_
     mi_assert_internal(delta >= 0 && mi_page_usable_block_size(page) >= (size - MI_PADDING_SIZE + delta));
     #endif
     mi_track_mem_defined(padding,sizeof(mi_padding_t));  // note: re-enable since mi_page_usable_block_size may set noaccess
-    padding->canary = (uint32_t)(mi_ptr_encode(page,block,page->keys));
+    padding->canary = mi_ptr_encode_canary(page,block,page->keys);
     padding->delta  = (uint32_t)(delta);
     #if MI_PADDING_CHECK
     if (!mi_page_is_huge(page)) {

src/free.c

@@ -414,7 +414,7 @@ static bool mi_page_decode_padding(const mi_page_t* page, const mi_block_t* bloc
   uintptr_t keys[2];
   keys[0] = page->keys[0];
   keys[1] = page->keys[1];
-  bool ok = ((uint32_t)mi_ptr_encode(page,block,keys) == canary && *delta <= *bsize);
+  bool ok = (mi_ptr_encode_canary(page,block,keys) == canary && *delta <= *bsize);
   mi_track_mem_noaccess(padding,sizeof(mi_padding_t));
   return ok;
 }

test/main-override-static.c

@@ -19,6 +19,7 @@ static void test_reserved(void);
 static void negative_stat(void);
 static void alloc_huge(void);
 static void test_heap_walk(void);
+static void test_canary_leak(void);
 // static void test_large_pages(void);
 
 
@@ -31,7 +32,8 @@ int main() {
   // double_free2();
   // corrupt_free();
   // block_overflow1();
-  block_overflow2();
+  // block_overflow2();
+  test_canary_leak();
   // test_aslr();
   // invalid_free();
   // test_reserved();
@@ -226,6 +228,15 @@ static void test_heap_walk(void) {
   mi_heap_visit_blocks(heap, true, &test_visit, NULL);
 }
 
+static void test_canary_leak(void) {
+  char* p = mi_mallocn_tp(char,23);
+  for(int i = 0; i < 23; i++) {
+    p[i] = '0'+i;
+  }
+  puts(p);
+  free(p);
+}
+
 // Experiment with huge OS pages
 #if 0