From 15a8e2ae22a417249643300cabdeb9db28679e09 Mon Sep 17 00:00:00 2001 From: Reuben Dunnington Date: Wed, 6 Sep 2023 22:55:14 -0700 Subject: [PATCH] wasm bindings: enforce pointer length * bindgen.py: print an error when a pointer argument doesn't have an annotated length * ensure all bindings with pointers have length annotations * add a couple helper functions for dealing with length annotations --- scripts/bindgen.py | 59 ++++++++++++++------------ src/runtime.c | 34 ++++++++++++--- src/wasmbind/core_api.json | 30 ++++++++----- src/wasmbind/gles_api_bind_manual.c | 35 ++++----------- src/wasmbind/io_api.json | 6 ++- src/wasmbind/surface_api.json | 9 ++-- src/wasmbind/surface_api_bind_manual.c | 11 +++++ 7 files changed, 109 insertions(+), 75 deletions(-) create mode 100644 src/wasmbind/surface_api_bind_manual.c diff --git a/scripts/bindgen.py b/scripts/bindgen.py index 96d05f3..0c92693 100755 --- a/scripts/bindgen.py +++ b/scripts/bindgen.py @@ -11,6 +11,11 @@ def needs_arg_ptr_stub(decl): res = True return(res) +def printError(str): + # This prints a string with a red foreground color. + # See this link for an explanation of console escape codes: https://stackabuse.com/how-to-print-colored-text-in-python/ + print("\x1b[38;5;196m" + "error: " + str + "\033[0;0m") + def bindgen(apiName, spec, **kwargs): guest_stubs_path = kwargs.get("guest_stubs") @@ -152,37 +157,39 @@ def bindgen(apiName, spec, **kwargs): argTag = arg['type']['tag'] argLen = arg.get('len') - if argTag == 'p' and argLen != None: - - s += '\t{\n' - s += '\t\tOC_ASSERT(((char*)'+ argName + ' >= (char*)_mem) && (((char*)'+ argName +' - (char*)_mem) < m3_GetMemorySize(runtime)), "parameter \''+argName+'\' is out of bounds");\n' - s += '\t\tOC_ASSERT((char*)' + argName + ' + ' - - proc = argLen.get('proc') - if proc != None: - s += proc + '(runtime, ' - lenProcArgs = argLen['args'] - for i, lenProcArg in enumerate(lenProcArgs): - s += lenProcArg - if i < len(lenProcArgs)-1: - s += ', ' - s += ')' + if argTag == 'p': + if argLen == None: + printError("binding '" + name + "' missing pointer length decoration for param '" + argName + "'") else: - components = argLen.get('components') - countArg = argLen.get('count') + s += '\t{\n' + s += '\t\tOC_ASSERT(((char*)'+ argName + ' >= (char*)_mem) && (((char*)'+ argName +' - (char*)_mem) < m3_GetMemorySize(runtime)), "parameter \''+argName+'\' is out of bounds");\n' + s += '\t\tOC_ASSERT((char*)' + argName + ' + ' - if components != None: - s += str(components) + proc = argLen.get('proc') + if proc != None: + s += proc + '(runtime, ' + lenProcArgs = argLen['args'] + for i, lenProcArg in enumerate(lenProcArgs): + s += lenProcArg + if i < len(lenProcArgs)-1: + s += ', ' + s += ')' + else: + components = argLen.get('components') + countArg = argLen.get('count') + + if components != None: + s += str(components) + if countArg != None: + s += '*' if countArg != None: - s += '*' - if countArg != None: - s += countArg + s += countArg - if typeCName.endswith('**') or (typeCName.startswith('void') == False and typeCName.startswith('const void') == False): - s += '*sizeof('+typeCName[:-1]+')' + if typeCName.endswith('**') or (typeCName.startswith('void') == False and typeCName.startswith('const void') == False): + s += '*sizeof('+typeCName[:-1]+')' - s += ' <= ((char*)_mem + m3_GetMemorySize(runtime)), "parameter \''+argName+'\' overflows wasm memory");\n' - s += '\t}\n' + s += ' <= ((char*)_mem + m3_GetMemorySize(runtime)), "parameter \''+argName+'\' overflows wasm memory");\n' + s += '\t}\n' s += '\t' diff --git a/src/runtime.c b/src/runtime.c index ac38895..e0f87cd 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -70,6 +70,25 @@ oc_str8 oc_runtime_get_wasm_memory() return (mem); } +u64 orca_check_cstring(IM3Runtime runtime, const char* ptr) +{ + uint32_t memorySize = 0; + char* memory = (char*)m3_GetMemory(runtime, &memorySize, 0); + + //NOTE: Here we are guaranteed that ptr is in [ memory ; memory + memorySize [ + // hence (memory + memorySize) - ptr is representable by size_t and <= memorySize + size_t maxLen = (memory + memorySize) - ptr; + + u64 len = strnlen(ptr, maxLen); + + if(len == maxLen) + { + //NOTE: string overflows wasm memory, return a length that will trigger the bounds check + len = maxLen + 1; + } + return (len + 1); //include null-terminator +} + void orca_wasm3_abort(IM3Runtime runtime, M3Result res, const char* file, const char* function, int line, const char* msg) { M3ErrorInfo errInfo = { 0 }; @@ -99,13 +118,13 @@ void oc_bridge_window_set_size(oc_vec2 size) } void oc_bridge_log(oc_log_level level, - int fileLen, - char* file, - int functionLen, - char* function, - int line, - int msgLen, - char* msg) + int fileLen, + char* file, + int functionLen, + char* function, + int line, + int msgLen, + char* msg) { oc_debug_overlay* debug = &__orcaApp.debugOverlay; @@ -349,6 +368,7 @@ void oc_wasm_env_init(oc_wasm_env* runtime) #include "wasmbind/gles_api_bind_manual.c" #include "wasmbind/gles_api_bind_gen.c" #include "wasmbind/io_api_bind_gen.c" +#include "wasmbind/surface_api_bind_manual.c" #include "wasmbind/surface_api_bind_gen.c" i32 orca_runloop(void* user) diff --git a/src/wasmbind/core_api.json b/src/wasmbind/core_api.json index 8d43000..566cfb8 100644 --- a/src/wasmbind/core_api.json +++ b/src/wasmbind/core_api.json @@ -8,17 +8,20 @@ {"name": "fileLen", "type": {"name": "int", "tag": "i"}}, {"name": "file", - "type": {"name": "char*", "tag": "p"}}, + "type": {"name": "char*", "tag": "p"}, + "len": {"count": "fileLen"}}, {"name": "functionLen", "type": {"name": "int", "tag": "i"}}, {"name": "function", - "type": {"name": "char*", "tag": "p"}}, + "type": {"name": "char*", "tag": "p"}, + "len": {"count": "functionLen"}}, {"name": "line", "type": {"name": "int", "tag": "i"}}, {"name": "msgLen", "type": {"name": "int", "tag": "i"}}, {"name": "msg", - "type": {"name": "char*", "tag": "p"}} + "type": {"name": "char*", "tag": "p"}, + "len": {"count": "msgLen"}} ] }, { @@ -33,15 +36,19 @@ "cname": "oc_assert_fail", "ret": {"name": "void", "tag": "v"}, "args": [ {"name": "file", - "type": {"name": "const char*", "tag": "p"}}, + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["file"]}}, {"name": "function", - "type": {"name": "const char*", "tag": "p"}}, + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["function"]}}, {"name": "line", "type": {"name": "int", "tag": "i"}}, {"name": "src", - "type": {"name": "const char*", "tag": "p"}}, + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["src"]}}, {"name": "note", - "type": {"name": "const char*", "tag": "p"}} + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["note"]}} ] }, { @@ -49,13 +56,16 @@ "cname": "oc_abort_ext", "ret": {"name": "void", "tag": "v"}, "args": [ {"name": "file", - "type": {"name": "const char*", "tag": "p"}}, + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["file"]}}, {"name": "function", - "type": {"name": "const char*", "tag": "p"}}, + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["function"]}}, {"name": "line", "type": {"name": "int", "tag": "i"}}, {"name": "note", - "type": {"name": "const char*", "tag": "p"}} + "type": {"name": "const char*", "tag": "p"}, + "len": {"proc": "orca_check_cstring", "args": ["note"]}} ] }, { diff --git a/src/wasmbind/gles_api_bind_manual.c b/src/wasmbind/gles_api_bind_manual.c index 90de2c4..6117b7a 100644 --- a/src/wasmbind/gles_api_bind_manual.c +++ b/src/wasmbind/gles_api_bind_manual.c @@ -3,25 +3,6 @@ // Manual pointer size checking functions //------------------------------------------------------------------------ -u64 orca_gles_check_cstring(IM3Runtime runtime, const char* ptr) -{ - uint32_t memorySize = 0; - char* memory = (char*)m3_GetMemory(runtime, &memorySize, 0); - - //NOTE: Here we are guaranteed that ptr is in [ memory ; memory + memorySize [ - // hence (memory + memorySize) - ptr is representable by size_t and <= memorySize - size_t maxLen = (memory + memorySize) - ptr; - - u64 len = strnlen(ptr, maxLen); - - if(len == maxLen) - { - //NOTE: string overflows wasm memory, return a length that will trigger the bounds check - len = maxLen + 1; - } - return (len + 1); //include null-terminator -} - u64 orca_gl_type_size(GLenum type) { u64 size = 8; @@ -772,37 +753,37 @@ u64 orca_glGetUniformuiv_params_length(IM3Runtime runtime, GLuint program, GLint u64 orca_glGetFragDataLocation_name_length(IM3Runtime runtime, const GLchar* name) { - return (orca_gles_check_cstring(runtime, name)); + return (orca_check_cstring(runtime, name)); } u64 orca_glGetUniformBlockIndex_uniformBlockName_length(IM3Runtime runtime, const GLchar* uniformBlockName) { - return (orca_gles_check_cstring(runtime, uniformBlockName)); + return (orca_check_cstring(runtime, uniformBlockName)); } u64 orca_glGetProgramResourceIndex_name_length(IM3Runtime runtime, const GLchar* name) { - return (orca_gles_check_cstring(runtime, name)); + return (orca_check_cstring(runtime, name)); } u64 orca_glGetProgramResourceLocation_name_length(IM3Runtime runtime, const GLchar* name) { - return (orca_gles_check_cstring(runtime, name)); + return (orca_check_cstring(runtime, name)); } u64 orca_glBindAttribLocation_name_length(IM3Runtime runtime, const GLchar* name) { - return (orca_gles_check_cstring(runtime, name)); + return (orca_check_cstring(runtime, name)); } u64 orca_glGetAttribLocation_name_length(IM3Runtime runtime, const GLchar* name) { - return (orca_gles_check_cstring(runtime, name)); + return (orca_check_cstring(runtime, name)); } u64 orca_glGetUniformLocation_name_length(IM3Runtime runtime, const GLchar* name) { - return (orca_gles_check_cstring(runtime, name)); + return (orca_check_cstring(runtime, name)); } //------------------------------------------------------------------------ @@ -967,7 +948,7 @@ const void* glGetUniformIndices_stub(IM3Runtime runtime, IM3ImportContext _ctx, char* raw = ((char*)_mem + uniformNames[i]); OC_ASSERT(raw >= (char*)_mem && (raw - (char*)_mem) < memorySize, "uniformName[%i] is out of bounds", i); - u64 len = orca_gles_check_cstring(runtime, raw); + u64 len = orca_check_cstring(runtime, raw); OC_ASSERT(raw + len <= ((char*)_mem + memorySize), "uniformName[%i] overflows wasm memory", i); diff --git a/src/wasmbind/io_api.json b/src/wasmbind/io_api.json index 5f720f3..4c8fb47 100644 --- a/src/wasmbind/io_api.json +++ b/src/wasmbind/io_api.json @@ -4,7 +4,8 @@ "cname": "oc_bridge_io_single_rect", "ret": {"name": "oc_io_cmp", "tag": "S"}, "args": [ {"name": "req", - "type": {"name": "oc_io_req*", "tag": "p"}}] + "type": {"name": "oc_io_req*", "tag": "p"}, + "len": {"components": 1}}] }, { "name": "oc_file_open_with_request", @@ -31,7 +32,8 @@ {"name": "flags", "type": {"name": "oc_file_open_flags", "tag": "i"}}, {"name": "desc", - "type": {"name": "oc_file_dialog_desc*", "cname": "oc_wasm_file_dialog_desc*", "tag": "p"}} + "type": {"name": "oc_file_dialog_desc*", "cname": "oc_wasm_file_dialog_desc*", "tag": "p"}, + "len": {"components": 1}} ] } ] diff --git a/src/wasmbind/surface_api.json b/src/wasmbind/surface_api.json index 633d7db..8065d32 100644 --- a/src/wasmbind/surface_api.json +++ b/src/wasmbind/surface_api.json @@ -34,7 +34,8 @@ {"name": "region", "type": {"name": "oc_rect", "tag": "S"}}, {"name": "pixels", - "type": {"name": "u8*", "tag": "p"}}] + "type": {"name": "u8*", "tag": "p"}, + "len": {"proc": "orca_image_upload_region_rgba8_length", "args": ["region"]}}] }, { "name": "oc_surface_get_size", @@ -96,11 +97,13 @@ {"name": "primitiveCount", "type": {"name": "u32", "tag": "i"}}, {"name": "primitives", - "type": {"name": "oc_primitive*", "tag": "p"}}, + "type": {"name": "oc_primitive*", "tag": "p"}, + "len": {"count": "primitiveCount"}}, {"name": "eltCount", "type": {"name": "u32", "tag": "i"}}, {"name": "elements", - "type": {"name": "oc_path_elt*", "tag": "p"}}] + "type": {"name": "oc_path_elt*", "tag": "p"}, + "len": {"count": "eltCount"}}] }, { "name": "oc_surface_canvas", diff --git a/src/wasmbind/surface_api_bind_manual.c b/src/wasmbind/surface_api_bind_manual.c new file mode 100644 index 0000000..0aaf714 --- /dev/null +++ b/src/wasmbind/surface_api_bind_manual.c @@ -0,0 +1,11 @@ + +//------------------------------------------------------------------------ +// image length checks +//------------------------------------------------------------------------ + +u64 orca_image_upload_region_rgba8_length(IM3Runtime runtime, oc_rect rect) +{ + u64 pixelFormatWidth = sizeof(u8) * 4; + u64 len = rect.w * rect.h * pixelFormatWidth; + return len; +}