From a40121aa599fff5ac91fdcff0cc69f89251f5c55 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 15 Aug 2024 23:51:55 +0900 Subject: [PATCH 1/4] native runtime: implement memory bounds check Fixes: https://github.com/aduros/wasm4/issues/750 https://github.com/aduros/wasm4/issues/709 --- runtimes/native/src/runtime.c | 71 ++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/runtimes/native/src/runtime.c b/runtimes/native/src/runtime.c index bf90321d..e466b4f3 100644 --- a/runtimes/native/src/runtime.c +++ b/runtimes/native/src/runtime.c @@ -1,6 +1,7 @@ #include "runtime.h" #include +#include #include #include "apu.h" @@ -40,6 +41,51 @@ static Memory* memory; static w4_Disk* disk; static bool firstFrame; +static void panic(const char *msg) +{ + /* REVISIT: it's cleaner to raise a wasm trap */ + fprintf(stderr, "fatal error in host function: %s\n", msg); + exit(1); +} + +static void out_of_bounds_access(void) +{ + panic("out of bands memory access"); +} + +static uint32_t mul_u32_with_overflow_check(uint32_t a, uint32_t b) +{ + uint32_t c = a * b; + if (c / a != b) { + panic("integer overflow"); + } + return c; +} + +static void bounds_check(const void *sp, size_t sz) +{ + const void *memory_sp = (const void *)memory; + const void *memory_ep = memory_sp + (1 << 16); + const void *ep = sp + sz; + if (ep <= sp || sp < memory_sp || memory_ep < ep) { + out_of_bounds_access(); + } +} + +static void bounds_check_cstr(const char *p) +{ + const void *memory_sp = (const void *)memory; + const void *memory_ep = memory_sp + (1 << 16); + if (p < memory_sp || memory_ep <= p) { + out_of_bounds_access(); + } + while (*p++ != 0) { + if (memory_ep <= p) { + out_of_bounds_access(); + } + } +} + void w4_runtimeInit (uint8_t* memoryBytes, w4_Disk* diskBytes) { memory = (Memory*)memoryBytes; disk = diskBytes; @@ -83,6 +129,9 @@ void w4_runtimeBlitSub (const uint8_t* sprite, int x, int y, int width, int heig bool flipX = (flags & 2); bool flipY = (flags & 4); bool rotate = (flags & 8); + uint32_t bpp = (int)bpp2 + 1; + uint32_t nbits = mul_u32_with_overflow_check(mul_u32_with_overflow_check(width, height), bpp); + bounds_check(sprite, nbits / 8); w4_framebufferBlit(sprite, x, y, width, height, srcX, srcY, stride, bpp2, flipX, flipY, rotate); } @@ -112,16 +161,19 @@ void w4_runtimeRect (int x, int y, int width, int height) { } void w4_runtimeText (const uint8_t* str, int x, int y) { + bounds_check_cstr(str); // printf("text: %s, %d, %d\n", str, x, y); w4_framebufferText(str, x, y); } void w4_runtimeTextUtf8 (const uint8_t* str, int byteLength, int x, int y) { + bounds_check(str, byteLength); // printf("textUtf8: %p, %d, %d, %d\n", str, byteLength, x, y); w4_framebufferTextUtf8(str, byteLength, x, y); } void w4_runtimeTextUtf16 (const uint16_t* str, int byteLength, int x, int y) { + bounds_check(str, byteLength); // printf("textUtf16: %p, %d, %d, %d\n", str, byteLength, x, y); w4_framebufferTextUtf16(str, byteLength, x, y); } @@ -132,6 +184,7 @@ void w4_runtimeTone (int frequency, int duration, int volume, int flags) { } int w4_runtimeDiskr (uint8_t* dest, int size) { + bounds_check(dest, size); if (!disk) { return 0; } @@ -144,6 +197,7 @@ int w4_runtimeDiskr (uint8_t* dest, int size) { } int w4_runtimeDiskw (const uint8_t* src, int size) { + bounds_check(src, size); if (!disk) { return 0; } @@ -157,20 +211,24 @@ int w4_runtimeDiskw (const uint8_t* src, int size) { } void w4_runtimeTrace (const uint8_t* str) { + bounds_check_cstr(str); puts(str); } void w4_runtimeTraceUtf8 (const uint8_t* str, int byteLength) { + bounds_check(str, byteLength); printf("%.*s\n", byteLength, str); } void w4_runtimeTraceUtf16 (const uint16_t* str, int byteLength) { + bounds_check(str, byteLength); printf("TODO: traceUtf16: %p, %d\n", str, byteLength); } void w4_runtimeTracef (const uint8_t* str, const void* stack) { const uint8_t* argPtr = stack; uint32_t strPtr; + bounds_check_cstr(str); for (; *str != 0; ++str) { if (*str == '%') { const uint8_t sym = *(++str); @@ -181,27 +239,30 @@ void w4_runtimeTracef (const uint8_t* str, const void* stack) { putc('%', stdout); break; case 'c': + bounds_check(argPtr, 4); putc(*(char*)argPtr, stdout); argPtr += 4; break; case 'd': + bounds_check(argPtr, 4); printf("%d", *(int32_t*)argPtr); argPtr += 4; break; case 'x': + bounds_check(argPtr, 4); printf("%x", *(uint32_t*)argPtr); argPtr += 4; break; case 's': + bounds_check(argPtr, 4); strPtr = *(uint32_t*)argPtr; argPtr += 4; - if (strPtr > 0 && strPtr < sizeof(*memory)) { - printf("%.*s", (int)(sizeof(*memory) - strPtr), (char*)memory + strPtr); - } else { - printf(""); - } + const char *strPtr_host = (const char *)memory + strPtr; + bounds_check_cstr(strPtr_host); + printf("%s", strPtr_host); break; case 'f': + bounds_check(argPtr, 8); printf("%lg", *(double*)argPtr); argPtr += 8; break; From 20027a60ceb12610c245b3a9c86676b779ab2bf9 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 15 Aug 2024 23:51:55 +0900 Subject: [PATCH 2/4] add some test code for memory bounds check --- test/integer_overflow/blit_1bpp_overflow.wat | 13 +++++++++++++ test/integer_overflow/blit_2bpp_overflow.wat | 13 +++++++++++++ test/out_of_bands_memory_access/blit_1bpp.wat | 13 +++++++++++++ test/out_of_bands_memory_access/blit_2bpp.wat | 13 +++++++++++++ test/out_of_bands_memory_access/tracef_double.wat | 10 ++++++++++ test/out_of_bands_memory_access/tracef_fmt.wat | 10 ++++++++++ test/out_of_bands_memory_access/tracef_str.wat | 15 +++++++++++++++ test/out_of_bands_memory_access/tracef_strptr.wat | 10 ++++++++++ 8 files changed, 97 insertions(+) create mode 100644 test/integer_overflow/blit_1bpp_overflow.wat create mode 100644 test/integer_overflow/blit_2bpp_overflow.wat create mode 100644 test/out_of_bands_memory_access/blit_1bpp.wat create mode 100644 test/out_of_bands_memory_access/blit_2bpp.wat create mode 100644 test/out_of_bands_memory_access/tracef_double.wat create mode 100644 test/out_of_bands_memory_access/tracef_fmt.wat create mode 100644 test/out_of_bands_memory_access/tracef_str.wat create mode 100644 test/out_of_bands_memory_access/tracef_strptr.wat diff --git a/test/integer_overflow/blit_1bpp_overflow.wat b/test/integer_overflow/blit_1bpp_overflow.wat new file mode 100644 index 00000000..b400d3a6 --- /dev/null +++ b/test/integer_overflow/blit_1bpp_overflow.wat @@ -0,0 +1,13 @@ +(module + (func $blit (import "env" "blit") (param i32 i32 i32 i32 i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 0 + i32.const 0 + i32.const 0 + i32.const 0x10 + i32.const 0x10000001 + i32.const 0 + call $blit + ) +) diff --git a/test/integer_overflow/blit_2bpp_overflow.wat b/test/integer_overflow/blit_2bpp_overflow.wat new file mode 100644 index 00000000..c453f4d5 --- /dev/null +++ b/test/integer_overflow/blit_2bpp_overflow.wat @@ -0,0 +1,13 @@ +(module + (func $blit (import "env" "blit") (param i32 i32 i32 i32 i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 0 + i32.const 0 + i32.const 0 + i32.const 0x8 + i32.const 0x10000001 + i32.const 1 + call $blit + ) +) diff --git a/test/out_of_bands_memory_access/blit_1bpp.wat b/test/out_of_bands_memory_access/blit_1bpp.wat new file mode 100644 index 00000000..e7910339 --- /dev/null +++ b/test/out_of_bands_memory_access/blit_1bpp.wat @@ -0,0 +1,13 @@ +(module + (func $blit (import "env" "blit") (param i32 i32 i32 i32 i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 65529 + i32.const 0 + i32.const 0 + i32.const 8 + i32.const 8 + i32.const 0 + call $blit + ) +) diff --git a/test/out_of_bands_memory_access/blit_2bpp.wat b/test/out_of_bands_memory_access/blit_2bpp.wat new file mode 100644 index 00000000..c3908822 --- /dev/null +++ b/test/out_of_bands_memory_access/blit_2bpp.wat @@ -0,0 +1,13 @@ +(module + (func $blit (import "env" "blit") (param i32 i32 i32 i32 i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 65521 + i32.const 0 + i32.const 0 + i32.const 8 + i32.const 8 + i32.const 1 + call $blit + ) +) diff --git a/test/out_of_bands_memory_access/tracef_double.wat b/test/out_of_bands_memory_access/tracef_double.wat new file mode 100644 index 00000000..15768f7d --- /dev/null +++ b/test/out_of_bands_memory_access/tracef_double.wat @@ -0,0 +1,10 @@ +(module + (func $tracef (import "env" "tracef") (param i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 50000 + i32.const 65529 + call $tracef + ) + (data (i32.const 50000) "%f\00") +) diff --git a/test/out_of_bands_memory_access/tracef_fmt.wat b/test/out_of_bands_memory_access/tracef_fmt.wat new file mode 100644 index 00000000..a8a81478 --- /dev/null +++ b/test/out_of_bands_memory_access/tracef_fmt.wat @@ -0,0 +1,10 @@ +(module + (func $tracef (import "env" "tracef") (param i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 65533 + i32.const 0 + call $tracef + ) + (data (i32.const 65533) "xxx") +) diff --git a/test/out_of_bands_memory_access/tracef_str.wat b/test/out_of_bands_memory_access/tracef_str.wat new file mode 100644 index 00000000..ba690c05 --- /dev/null +++ b/test/out_of_bands_memory_access/tracef_str.wat @@ -0,0 +1,15 @@ +(module + (func $tracef (import "env" "tracef") (param i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 40000 + i32.const 65533 + i32.store + + i32.const 50000 + i32.const 40000 + call $tracef + ) + (data (i32.const 50000) "%s\00") + (data (i32.const 65533) "xxx") +) diff --git a/test/out_of_bands_memory_access/tracef_strptr.wat b/test/out_of_bands_memory_access/tracef_strptr.wat new file mode 100644 index 00000000..52e9da11 --- /dev/null +++ b/test/out_of_bands_memory_access/tracef_strptr.wat @@ -0,0 +1,10 @@ +(module + (func $tracef (import "env" "tracef") (param i32 i32)) + (memory (import "env" "memory") 1 1) + (func (export "update") + i32.const 50000 + i32.const 65533 + call $tracef + ) + (data (i32.const 50000) "%s\00") +) From 526984ea846813d73a0c36488fbde9ffce4ea136 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 15 Aug 2024 23:51:55 +0900 Subject: [PATCH 3/4] avoid void pointer arithmetics, which is a GCC extention --- runtimes/native/src/runtime.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runtimes/native/src/runtime.c b/runtimes/native/src/runtime.c index e466b4f3..5422ae48 100644 --- a/runtimes/native/src/runtime.c +++ b/runtimes/native/src/runtime.c @@ -65,8 +65,8 @@ static uint32_t mul_u32_with_overflow_check(uint32_t a, uint32_t b) static void bounds_check(const void *sp, size_t sz) { const void *memory_sp = (const void *)memory; - const void *memory_ep = memory_sp + (1 << 16); - const void *ep = sp + sz; + const void *memory_ep = (const uint8_t *)memory_sp + (1 << 16); + const void *ep = (const uint8_t *)sp + sz; if (ep <= sp || sp < memory_sp || memory_ep < ep) { out_of_bounds_access(); } @@ -75,7 +75,7 @@ static void bounds_check(const void *sp, size_t sz) static void bounds_check_cstr(const char *p) { const void *memory_sp = (const void *)memory; - const void *memory_ep = memory_sp + (1 << 16); + const void *memory_ep = (const uint8_t *)memory_sp + (1 << 16); if (p < memory_sp || memory_ep <= p) { out_of_bounds_access(); } From 380805fd07ca43a129734a1a71df01839cafde36 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 15 Aug 2024 23:51:55 +0900 Subject: [PATCH 4/4] runtimes/native: don't reject zero-byte memory access eg. zero byte textUtf8 is ok. --- runtimes/native/src/runtime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtimes/native/src/runtime.c b/runtimes/native/src/runtime.c index 5422ae48..e710f9e6 100644 --- a/runtimes/native/src/runtime.c +++ b/runtimes/native/src/runtime.c @@ -67,7 +67,7 @@ static void bounds_check(const void *sp, size_t sz) const void *memory_sp = (const void *)memory; const void *memory_ep = (const uint8_t *)memory_sp + (1 << 16); const void *ep = (const uint8_t *)sp + sz; - if (ep <= sp || sp < memory_sp || memory_ep < ep) { + if (ep < sp || sp < memory_sp || memory_ep < ep) { out_of_bounds_access(); } }