From 59b2099b68ae3e027f2787397226e503501e38b8 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Thu, 27 Jul 2023 21:53:48 +0800 Subject: [PATCH] Fix some check issues on table operations (#2392) Fix some check issues on table.init, table.fill and table.copy, and unify the check method for all running modes. Fix issue #2390 and #2096. --- core/iwasm/aot/aot_runtime.c | 15 +++---- core/iwasm/fast-jit/fe/jit_emit_table.c | 41 ++++++++++---------- core/iwasm/interpreter/wasm.h | 19 ++++++++- core/iwasm/interpreter/wasm_interp_classic.c | 36 ++++++++--------- core/iwasm/interpreter/wasm_interp_fast.c | 30 ++++++++------ core/iwasm/interpreter/wasm_runtime.c | 15 +++---- 6 files changed, 91 insertions(+), 65 deletions(-) diff --git a/core/iwasm/aot/aot_runtime.c b/core/iwasm/aot/aot_runtime.c index 35735b94e..e0d56a698 100644 --- a/core/iwasm/aot/aot_runtime.c +++ b/core/iwasm/aot/aot_runtime.c @@ -2489,13 +2489,13 @@ aot_table_init(AOTModuleInstance *module_inst, uint32 tbl_idx, tbl_seg = module->table_init_data_list[tbl_seg_idx]; bh_assert(tbl_seg); - if (!length) { + if (offset_len_out_of_bounds(src_offset, length, tbl_seg->func_index_count) + || offset_len_out_of_bounds(dst_offset, length, tbl_inst->cur_size)) { + aot_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); return; } - if (length + src_offset > tbl_seg->func_index_count - || dst_offset + length > tbl_inst->cur_size) { - aot_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); + if (!length) { return; } @@ -2528,8 +2528,9 @@ aot_table_copy(AOTModuleInstance *module_inst, uint32 src_tbl_idx, dst_tbl_inst = module_inst->tables[dst_tbl_idx]; bh_assert(dst_tbl_inst); - if ((uint64)dst_offset + length > dst_tbl_inst->cur_size - || (uint64)src_offset + length > src_tbl_inst->cur_size) { + if (offset_len_out_of_bounds(dst_offset, length, dst_tbl_inst->cur_size) + || offset_len_out_of_bounds(src_offset, length, + src_tbl_inst->cur_size)) { aot_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); return; } @@ -2554,7 +2555,7 @@ aot_table_fill(AOTModuleInstance *module_inst, uint32 tbl_idx, uint32 length, tbl_inst = module_inst->tables[tbl_idx]; bh_assert(tbl_inst); - if (data_offset + length > tbl_inst->cur_size) { + if (offset_len_out_of_bounds(data_offset, length, tbl_inst->cur_size)) { aot_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); return; } diff --git a/core/iwasm/fast-jit/fe/jit_emit_table.c b/core/iwasm/fast-jit/fe/jit_emit_table.c index 9fb61931f..ea1b33883 100644 --- a/core/iwasm/fast-jit/fe/jit_emit_table.c +++ b/core/iwasm/fast-jit/fe/jit_emit_table.c @@ -88,27 +88,28 @@ fail: static int wasm_init_table(WASMModuleInstance *inst, uint32 tbl_idx, uint32 elem_idx, - uint32 dst, uint32 len, uint32 src) + uint32 dst_offset, uint32 len, uint32 src_offset) { WASMTableInstance *tbl; uint32 tbl_sz; WASMTableSeg *elem; uint32 elem_len; - tbl = inst->tables[tbl_idx]; - tbl_sz = tbl->cur_size; - if (dst > tbl_sz || tbl_sz - dst < len) - goto out_of_bounds; - elem = inst->module->table_segments + elem_idx; elem_len = elem->function_count; - if (src > elem_len || elem_len - src < len) + if (offset_len_out_of_bounds(src_offset, len, elem_len)) + goto out_of_bounds; + + tbl = inst->tables[tbl_idx]; + tbl_sz = tbl->cur_size; + if (offset_len_out_of_bounds(dst_offset, len, tbl_sz)) goto out_of_bounds; bh_memcpy_s((uint8 *)tbl + offsetof(WASMTableInstance, elems) - + dst * sizeof(uint32), - (uint32)((tbl_sz - dst) * sizeof(uint32)), - elem->func_indexes + src, (uint32)(len * sizeof(uint32))); + + dst_offset * sizeof(uint32), + (uint32)((tbl_sz - dst_offset) * sizeof(uint32)), + elem->func_indexes + src_offset, + (uint32)(len * sizeof(uint32))); return 0; out_of_bounds: @@ -157,14 +158,14 @@ wasm_copy_table(WASMModuleInstance *inst, uint32 src_tbl_idx, WASMTableInstance *src_tbl, *dst_tbl; uint32 src_tbl_sz, dst_tbl_sz; - src_tbl = inst->tables[src_tbl_idx]; - src_tbl_sz = src_tbl->cur_size; - if (src_offset > src_tbl_sz || src_tbl_sz - src_offset < len) - goto out_of_bounds; - dst_tbl = inst->tables[dst_tbl_idx]; dst_tbl_sz = dst_tbl->cur_size; - if (dst_offset > dst_tbl_sz || dst_tbl_sz - dst_offset < len) + if (offset_len_out_of_bounds(dst_offset, len, dst_tbl_sz)) + goto out_of_bounds; + + src_tbl = inst->tables[src_tbl_idx]; + src_tbl_sz = src_tbl->cur_size; + if (offset_len_out_of_bounds(src_offset, len, src_tbl_sz)) goto out_of_bounds; bh_memmove_s((uint8 *)dst_tbl + offsetof(WASMTableInstance, elems) @@ -263,7 +264,7 @@ fail: } static int -wasm_fill_table(WASMModuleInstance *inst, uint32 tbl_idx, uint32 dst, +wasm_fill_table(WASMModuleInstance *inst, uint32 tbl_idx, uint32 dst_offset, uint32 val, uint32 len) { WASMTableInstance *tbl; @@ -272,11 +273,11 @@ wasm_fill_table(WASMModuleInstance *inst, uint32 tbl_idx, uint32 dst, tbl = inst->tables[tbl_idx]; tbl_sz = tbl->cur_size; - if (dst > tbl_sz || tbl_sz - dst < len) + if (offset_len_out_of_bounds(dst_offset, len, tbl_sz)) goto out_of_bounds; - for (; len != 0; dst++, len--) { - tbl->elems[dst] = val; + for (; len != 0; dst_offset++, len--) { + tbl->elems[dst_offset] = val; } return 0; diff --git a/core/iwasm/interpreter/wasm.h b/core/iwasm/interpreter/wasm.h index 0797a018b..c7d9f1de9 100644 --- a/core/iwasm/interpreter/wasm.h +++ b/core/iwasm/interpreter/wasm.h @@ -627,7 +627,6 @@ typedef struct WASMBranchBlock { uint32 cell_num; } WASMBranchBlock; -/* Execution environment, e.g. stack info */ /** * Align an unsigned value on a alignment boundary. * @@ -643,6 +642,24 @@ align_uint(unsigned v, unsigned b) return (v + m) & ~m; } +/** + * Check whether a piece of data is out of range + * + * @param offset the offset that the data starts + * @param len the length of the data + * @param max_size the maximum size of the data range + * + * @return true if out of range, false otherwise + */ +inline static bool +offset_len_out_of_bounds(uint32 offset, uint32 len, uint32 max_size) +{ + if (offset + len < offset /* integer overflow */ + || offset + len > max_size) + return true; + return false; +} + /** * Return the hash value of c string. */ diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 55fca2921..23e165bea 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -3247,7 +3247,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, case WASM_OP_TABLE_INIT: { uint32 tbl_idx, elem_idx; - uint64 n, s, d; + uint32 n, s, d; WASMTableInstance *tbl_inst; read_leb_uint32(frame_ip, frame_ip_end, elem_idx); @@ -3262,20 +3262,21 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, s = (uint32)POP_I32(); d = (uint32)POP_I32(); - /* TODO: what if the element is not passive? */ - - if (!n) { - break; - } - - if (n + s > module->module->table_segments[elem_idx] - .function_count - || d + n > tbl_inst->cur_size) { + if (offset_len_out_of_bounds( + s, n, + module->module->table_segments[elem_idx] + .function_count) + || offset_len_out_of_bounds(d, n, + tbl_inst->cur_size)) { wasm_set_exception(module, "out of bounds table access"); goto got_exception; } + if (!n) { + break; + } + if (module->module->table_segments[elem_idx] .is_dropped) { wasm_set_exception(module, @@ -3316,7 +3317,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, case WASM_OP_TABLE_COPY: { uint32 src_tbl_idx, dst_tbl_idx; - uint64 n, s, d; + uint32 n, s, d; WASMTableInstance *src_tbl_inst, *dst_tbl_inst; read_leb_uint32(frame_ip, frame_ip_end, dst_tbl_idx); @@ -3333,8 +3334,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, s = (uint32)POP_I32(); d = (uint32)POP_I32(); - if (d + n > dst_tbl_inst->cur_size - || s + n > src_tbl_inst->cur_size) { + if (offset_len_out_of_bounds(d, n, + dst_tbl_inst->cur_size) + || offset_len_out_of_bounds( + s, n, src_tbl_inst->cur_size)) { wasm_set_exception(module, "out of bounds table access"); goto got_exception; @@ -3404,11 +3407,8 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, fill_val = POP_I32(); i = POP_I32(); - /* TODO: what if the element is not passive? */ - /* TODO: what if the element is dropped? */ - - if (i + n > tbl_inst->cur_size) { - /* TODO: verify warning content */ + if (offset_len_out_of_bounds(i, n, + tbl_inst->cur_size)) { wasm_set_exception(module, "out of bounds table access"); goto got_exception; diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index 070fcdbee..458eb2e44 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -3078,7 +3078,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, case WASM_OP_TABLE_INIT: { uint32 tbl_idx, elem_idx; - uint64 n, s, d; + uint32 n, s, d; WASMTableInstance *tbl_inst; elem_idx = read_uint32(frame_ip); @@ -3093,18 +3093,21 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, s = (uint32)POP_I32(); d = (uint32)POP_I32(); - if (!n) { - break; - } - - if (n + s > module->module->table_segments[elem_idx] - .function_count - || d + n > tbl_inst->cur_size) { + if (offset_len_out_of_bounds( + s, n, + module->module->table_segments[elem_idx] + .function_count) + || offset_len_out_of_bounds(d, n, + tbl_inst->cur_size)) { wasm_set_exception(module, "out of bounds table access"); goto got_exception; } + if (!n) { + break; + } + if (module->module->table_segments[elem_idx] .is_dropped) { wasm_set_exception(module, @@ -3143,7 +3146,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, case WASM_OP_TABLE_COPY: { uint32 src_tbl_idx, dst_tbl_idx; - uint64 n, s, d; + uint32 n, s, d; WASMTableInstance *src_tbl_inst, *dst_tbl_inst; dst_tbl_idx = read_uint32(frame_ip); @@ -3160,8 +3163,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, s = (uint32)POP_I32(); d = (uint32)POP_I32(); - if (d + n > dst_tbl_inst->cur_size - || s + n > src_tbl_inst->cur_size) { + if (offset_len_out_of_bounds(d, n, + dst_tbl_inst->cur_size) + || offset_len_out_of_bounds( + s, n, src_tbl_inst->cur_size)) { wasm_set_exception(module, "out of bounds table access"); goto got_exception; @@ -3232,7 +3237,8 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, fill_val = POP_I32(); i = POP_I32(); - if (i + n > tbl_inst->cur_size) { + if (offset_len_out_of_bounds(i, n, + tbl_inst->cur_size)) { wasm_set_exception(module, "out of bounds table access"); goto got_exception; diff --git a/core/iwasm/interpreter/wasm_runtime.c b/core/iwasm/interpreter/wasm_runtime.c index 00f0cf668..84155a05a 100644 --- a/core/iwasm/interpreter/wasm_runtime.c +++ b/core/iwasm/interpreter/wasm_runtime.c @@ -3301,13 +3301,13 @@ llvm_jit_table_init(WASMModuleInstance *module_inst, uint32 tbl_idx, bh_assert(tbl_inst); bh_assert(tbl_seg); - if (!length) { + if (offset_len_out_of_bounds(src_offset, length, tbl_seg->function_count) + || offset_len_out_of_bounds(dst_offset, length, tbl_inst->cur_size)) { + jit_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); return; } - if (length + src_offset > tbl_seg->function_count - || dst_offset + length > tbl_inst->cur_size) { - jit_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); + if (!length) { return; } @@ -3349,8 +3349,9 @@ llvm_jit_table_copy(WASMModuleInstance *module_inst, uint32 src_tbl_idx, bh_assert(src_tbl_inst); bh_assert(dst_tbl_inst); - if ((uint64)dst_offset + length > dst_tbl_inst->cur_size - || (uint64)src_offset + length > src_tbl_inst->cur_size) { + if (offset_len_out_of_bounds(dst_offset, length, dst_tbl_inst->cur_size) + || offset_len_out_of_bounds(src_offset, length, + src_tbl_inst->cur_size)) { jit_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); return; } @@ -3382,7 +3383,7 @@ llvm_jit_table_fill(WASMModuleInstance *module_inst, uint32 tbl_idx, tbl_inst = wasm_get_table_inst(module_inst, tbl_idx); bh_assert(tbl_inst); - if (data_offset + length > tbl_inst->cur_size) { + if (offset_len_out_of_bounds(data_offset, length, tbl_inst->cur_size)) { jit_set_exception_with_id(module_inst, EXCE_OUT_OF_BOUNDS_TABLE_ACCESS); return; }