Skip to content

Commit aac78d3

Browse files
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 Signed-off-by: Tavi <tavi@divested.dev>
1 parent 4fe9018 commit aac78d3

17 files changed

+253
-2
lines changed

Android.bp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ common_cflags = [
2828
"-DN_ARENA=1",
2929
"-DCONFIG_STATS=true",
3030
"-DCONFIG_SELF_INIT=false",
31+
"-DCONFIG_BLOCK_OPS_CHECK_SIZE=false",
3132
]
3233

3334
cc_defaults {

CREDITS

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ h_malloc.c open-addressed hash table (regions_grow, regions_insert, regions_find
2323
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
2424
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
2525

26+
h_malloc.c block operations (h_memcpy_real, h_memmove_real, h_memset_real):
27+
28+
Copyright (C) 2022, 2023 struct <chris.rohlf@gmail.com>
29+
Copyright (C) 2023 David Carlier <devnexen@gmail.com>
30+
Apache-2.0
31+
2632
libdivide:
2733

2834
Copyright (C) 2010 - 2019 ridiculous_fish, <libdivide@ridiculousfish.com>

Makefile

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ ifeq (,$(filter $(CONFIG_SELF_INIT),true false))
8989
$(error CONFIG_SELF_INIT must be true or false)
9090
endif
9191

92+
ifeq (,$(filter $(CONFIG_BLOCK_OPS_CHECK_SIZE),true false))
93+
$(error CONFIG_BLOCK_OPS_CHECK_SIZE must be true or false)
94+
endif
95+
9296
CPPFLAGS += \
9397
-DCONFIG_SEAL_METADATA=$(CONFIG_SEAL_METADATA) \
9498
-DZERO_ON_FREE=$(CONFIG_ZERO_ON_FREE) \
@@ -108,7 +112,8 @@ CPPFLAGS += \
108112
-DCONFIG_CLASS_REGION_SIZE=$(CONFIG_CLASS_REGION_SIZE) \
109113
-DN_ARENA=$(CONFIG_N_ARENA) \
110114
-DCONFIG_STATS=$(CONFIG_STATS) \
111-
-DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT)
115+
-DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT) \
116+
-DCONFIG_BLOCK_OPS_CHECK_SIZE=$(CONFIG_BLOCK_OPS_CHECK_SIZE)
112117

113118
$(OUT)/libhardened_malloc$(SUFFIX).so: $(OBJECTS) | $(OUT)
114119
$(CC) $(CFLAGS) $(LDFLAGS) -shared $^ $(LDLIBS) -o $@

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ The following boolean configuration options are available:
276276
hardware, which may become drastically lower in the future. Whether or not
277277
this feature is enabled, the metadata is all contained within an isolated
278278
memory region with high entropy random guard regions around it.
279+
* `CONFIG_BLOCK_OPS_CHECK_SIZE`: `true` (default) or `false` to ensure length
280+
parameter of the memcpy/memmove/memset block operations are within
281+
approximate bounds to minimize buffer overflows. Note, memset override is
282+
currently disabled due to improper behavior.
279283

280284
The following integer configuration options are available:
281285

config/default.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
2121
CONFIG_N_ARENA := 4
2222
CONFIG_STATS := false
2323
CONFIG_SELF_INIT := true
24+
CONFIG_BLOCK_OPS_CHECK_SIZE := true

config/light.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
2121
CONFIG_N_ARENA := 4
2222
CONFIG_STATS := false
2323
CONFIG_SELF_INIT := true
24+
CONFIG_BLOCK_OPS_CHECK_SIZE := false

h_malloc.c

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,6 +1874,80 @@ EXPORT size_t h_malloc_object_size_fast(const void *p) {
18741874
return SIZE_MAX;
18751875
}
18761876

1877+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
1878+
void *h_memcpy_real(void *dst, const void *src, size_t len) {
1879+
char *p_dst = (char *)dst;
1880+
char const *p_src = (char const *)src;
1881+
1882+
while(len--) {
1883+
*p_dst++ = *p_src++;
1884+
}
1885+
1886+
return dst;
1887+
}
1888+
1889+
EXPORT void *h_memcpy(void *dst, const void *src, size_t len) {
1890+
if (len > malloc_object_size(src)) {
1891+
fatal_error("memcpy read overflow");
1892+
}
1893+
if (len > malloc_object_size(dst)) {
1894+
fatal_error("memcpy buffer overflow");
1895+
}
1896+
1897+
return h_memcpy_real(dst, src, len);
1898+
}
1899+
1900+
void *h_memmove_real(void *dst, const void *src, size_t len) {
1901+
char *p_dst = (char *)dst;
1902+
char const *p_src = (char const *)src;
1903+
1904+
if(dst == src) {
1905+
return dst;
1906+
}
1907+
1908+
if(p_src < p_dst) {
1909+
p_dst += len;
1910+
p_src += len;
1911+
while(len--) {
1912+
*--p_dst = *--p_src;
1913+
}
1914+
} else {
1915+
dst = h_memcpy(dst, src, len);
1916+
}
1917+
1918+
return dst;
1919+
}
1920+
1921+
EXPORT void *h_memmove(void *dst, const void *src, size_t len) {
1922+
if (len > malloc_object_size(src)) {
1923+
fatal_error("memmove read overflow");
1924+
}
1925+
if (len > malloc_object_size(dst)) {
1926+
fatal_error("memmove buffer overflow");
1927+
}
1928+
1929+
return h_memmove_real(dst, src, len);
1930+
}
1931+
1932+
void *h_memset_real(void *dst, int value, size_t len) {
1933+
char *p_dst = (char *)dst;
1934+
1935+
while(len--) {
1936+
*p_dst++ = value;
1937+
}
1938+
1939+
return dst;
1940+
}
1941+
1942+
EXPORT void *h_memset(void *dst, int value, size_t len) {
1943+
if (len > malloc_object_size(dst)) {
1944+
fatal_error("memset buffer overflow");
1945+
}
1946+
1947+
return h_memset_real(dst, value, len);
1948+
}
1949+
#endif
1950+
18771951
EXPORT int h_mallopt(UNUSED int param, UNUSED int value) {
18781952
#ifdef __ANDROID__
18791953
if (param == M_PURGE) {

include/h_malloc.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ extern "C" {
1515
#define h_realloc realloc
1616
#define h_aligned_alloc aligned_alloc
1717
#define h_free free
18+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
19+
#define h_memcpy memcpy
20+
#define h_memmove memmove
21+
//#define h_memset memset
22+
#endif
1823

1924
#define h_posix_memalign posix_memalign
2025

@@ -54,6 +59,14 @@ __attribute__((alloc_size(2))) void *h_realloc(void *ptr, size_t size);
5459
__attribute__((malloc)) __attribute__((alloc_size(2))) __attribute__((alloc_align(1)))
5560
void *h_aligned_alloc(size_t alignment, size_t size);
5661
void h_free(void *ptr);
62+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
63+
void *h_memcpy_real(void *dst, const void *src, size_t len);
64+
void *h_memcpy(void *dst, const void *src, size_t len);
65+
void *h_memmove_real(void *dst, const void *src, size_t len);
66+
void *h_memmove(void *dst, const void *src, size_t len);
67+
void *h_memset_real(void *dst, int value, size_t len);
68+
void *h_memset(void *dst, int value, size_t len);
69+
#endif
5770

5871
// POSIX
5972
int h_posix_memalign(void **memptr, size_t alignment, size_t size);

test/.gitignore

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,10 @@ overflow_small_8_byte
4141
uninitialized_read_large
4242
uninitialized_read_small
4343
realloc_init
44+
memcpy_buffer_overflow
45+
memcpy_read_overflow
46+
memcpy_valid
47+
memmove_buffer_overflow
48+
memmove_read_overflow
49+
memmove_valid
4450
__pycache__/

test/Makefile

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,13 @@ EXECUTABLES := \
6767
invalid_malloc_object_size_small \
6868
invalid_malloc_object_size_small_quarantine \
6969
impossibly_large_malloc \
70-
realloc_init
70+
realloc_init \
71+
memcpy_buffer_overflow \
72+
memcpy_read_overflow \
73+
memcpy_valid \
74+
memmove_buffer_overflow \
75+
memmove_read_overflow \
76+
memmove_valid
7177

7278
all: $(EXECUTABLES)
7379

test/memcpy_buffer_overflow.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include <stdio.h>
2+
#include <stdlib.h>
3+
#include <string.h>
4+
5+
#include "test_util.h"
6+
7+
OPTNONE int main(void) {
8+
char *firstbuffer = malloc(16);
9+
char *secondbuffer = malloc(32);
10+
if (!firstbuffer && !secondbuffer) {
11+
return 1;
12+
}
13+
memset(secondbuffer, 'a', 16);
14+
memcpy(firstbuffer, secondbuffer, 32);
15+
return 1;
16+
}

test/memcpy_read_overflow.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include <stdio.h>
2+
#include <stdlib.h>
3+
#include <string.h>
4+
5+
#include "test_util.h"
6+
7+
OPTNONE int main(void) {
8+
char *firstbuffer = malloc(32);
9+
char *secondbuffer = malloc(16);
10+
if (!firstbuffer && !secondbuffer) {
11+
return 1;
12+
}
13+
memset(secondbuffer, 'a', 16);
14+
memcpy(firstbuffer, secondbuffer, 32);
15+
return 1;
16+
}

test/memcpy_valid.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include <stdio.h>
2+
#include <stdlib.h>
3+
#include <string.h>
4+
5+
#include "test_util.h"
6+
7+
OPTNONE int main(void) {
8+
char *firstbuffer = malloc(16);
9+
char *secondbuffer = malloc(16);
10+
if (!firstbuffer && !secondbuffer) {
11+
return 1;
12+
}
13+
memset(secondbuffer, 'a', 16);
14+
memcpy(firstbuffer, secondbuffer, 16);
15+
return 0;
16+
}

test/memmove_buffer_overflow.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include <stdio.h>
2+
#include <stdlib.h>
3+
#include <string.h>
4+
5+
#include "test_util.h"
6+
7+
OPTNONE int main(void) {
8+
char *firstbuffer = malloc(16);
9+
char *secondbuffer = malloc(32);
10+
if (!firstbuffer && !secondbuffer) {
11+
return 1;
12+
}
13+
memset(secondbuffer, 'a', 16);
14+
memmove(firstbuffer, secondbuffer, 32);
15+
return 1;
16+
}

test/memmove_read_overflow.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include <stdio.h>
2+
#include <stdlib.h>
3+
#include <string.h>
4+
5+
#include "test_util.h"
6+
7+
OPTNONE int main(void) {
8+
char *firstbuffer = malloc(32);
9+
char *secondbuffer = malloc(16);
10+
if (!firstbuffer && !secondbuffer) {
11+
return 1;
12+
}
13+
memset(secondbuffer, 'a', 16);
14+
memmove(firstbuffer, secondbuffer, 32);
15+
return 1;
16+
}

test/memmove_valid.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include <stdio.h>
2+
#include <stdlib.h>
3+
#include <string.h>
4+
5+
#include "test_util.h"
6+
7+
OPTNONE int main(void) {
8+
char *firstbuffer = malloc(16);
9+
char *secondbuffer = malloc(16);
10+
if (!firstbuffer && !secondbuffer) {
11+
return 1;
12+
}
13+
memset(secondbuffer, 'a', 16);
14+
memmove(firstbuffer, secondbuffer, 16);
15+
return 0;
16+
}

test/test_smc.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,5 +238,43 @@ def test_realloc_init(self):
238238
"realloc_init")
239239
self.assertEqual(returncode, 0)
240240

241+
def test_memcpy_buffer_overflow(self):
242+
_stdout, stderr, returncode = self.run_test(
243+
"memcpy_buffer_overflow")
244+
self.assertEqual(returncode, -6)
245+
self.assertEqual(stderr.decode(
246+
"utf-8"), "fatal allocator error: memcpy buffer overflow\n")
247+
248+
def test_memcpy_read_overflow(self):
249+
_stdout, stderr, returncode = self.run_test(
250+
"memcpy_read_overflow")
251+
self.assertEqual(returncode, -6)
252+
self.assertEqual(stderr.decode(
253+
"utf-8"), "fatal allocator error: memcpy read overflow\n")
254+
255+
def test_memcpy_valid(self):
256+
_stdout, _stderr, returncode = self.run_test(
257+
"memcpy_valid")
258+
self.assertEqual(returncode, 0)
259+
260+
def test_memmove_buffer_overflow(self):
261+
_stdout, stderr, returncode = self.run_test(
262+
"memmove_buffer_overflow")
263+
self.assertEqual(returncode, -6)
264+
self.assertEqual(stderr.decode(
265+
"utf-8"), "fatal allocator error: memmove buffer overflow\n")
266+
267+
def test_memmove_read_overflow(self):
268+
_stdout, stderr, returncode = self.run_test(
269+
"memmove_read_overflow")
270+
self.assertEqual(returncode, -6)
271+
self.assertEqual(stderr.decode(
272+
"utf-8"), "fatal allocator error: memmove read overflow\n")
273+
274+
def test_memmove_valid(self):
275+
_stdout, _stderr, returncode = self.run_test(
276+
"memmove_valid")
277+
self.assertEqual(returncode, 0)
278+
241279
if __name__ == '__main__':
242280
unittest.main()

0 commit comments

Comments
 (0)