Fix read and validation of misc/simd/atomic sub opcodes (#3115)

The format of sub opcodes after misc, simd and atomic prefix is leb u32. The issue was found in #2921.
bytecodealliance · Feb 2, 2024 · 2eb6006 · 2eb6006
1 parent b3f728c
commit 2eb6006
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 28 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -24,6 +24,7 @@
 - fast-interp: Fix frame_offset pop order (#3101)
 - Fix AOT compilation on MacOS (#3102)
 - fast-interp: Fix block with parameter in polymorphic stack issue (#3112)
+- Fix read and validation of misc/simd/atomic sub opcodes (#3115)
 
 ### Enhancements
 - Clear compilation warning and dead code (#3002)

diff --git a/core/iwasm/compilation/aot_compiler.c b/core/iwasm/compilation/aot_compiler.c
@@ -1050,7 +1050,9 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index)
                 uint32 opcode1;
 
                 read_leb_uint32(frame_ip, frame_ip_end, opcode1);
-                opcode = (uint32)opcode1;
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
+                opcode = (uint8)opcode1;
 
 #if WASM_ENABLE_BULK_MEMORY != 0
                 if (WASM_OP_MEMORY_INIT <= opcode
@@ -1211,10 +1213,13 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index)
             case WASM_OP_ATOMIC_PREFIX:
             {
                 uint8 bin_op, op_type;
+                uint32 opcode1;
+
+                read_leb_uint32(frame_ip, frame_ip_end, opcode1);
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
+                opcode = (uint8)opcode1;
 
-                if (frame_ip < frame_ip_end) {
-                    opcode = *frame_ip++;
-                }
                 if (opcode != WASM_OP_ATOMIC_FENCE) {
                     read_leb_uint32(frame_ip, frame_ip_end, align);
                     read_leb_uint32(frame_ip, frame_ip_end, offset);
@@ -1364,11 +1369,17 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index)
 #if WASM_ENABLE_SIMD != 0
             case WASM_OP_SIMD_PREFIX:
             {
+                uint32 opcode1;
+
                 if (!comp_ctx->enable_simd) {
                     goto unsupport_simd;
                 }
 
-                opcode = *frame_ip++;
+                read_leb_uint32(frame_ip, frame_ip_end, opcode1);
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
+                opcode = (uint8)opcode1;
+
                 /* follow the order of enum WASMSimdEXTOpcode in
                    wasm_opcode.h */
                 switch (opcode) {

diff --git a/core/iwasm/fast-jit/jit_frontend.c b/core/iwasm/fast-jit/jit_frontend.c
@@ -2257,7 +2257,9 @@ jit_compile_func(JitCompContext *cc)
                 uint32 opcode1;
 
                 read_leb_uint32(frame_ip, frame_ip_end, opcode1);
-                opcode = (uint32)opcode1;
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
+                opcode = (uint8)opcode1;
 
                 switch (opcode) {
                     case WASM_OP_I32_TRUNC_SAT_S_F32:
@@ -2396,10 +2398,13 @@ jit_compile_func(JitCompContext *cc)
             case WASM_OP_ATOMIC_PREFIX:
             {
                 uint8 bin_op, op_type;
+                uint32 opcode1;
+
+                read_leb_uint32(frame_ip, frame_ip_end, opcode1);
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
+                opcode = (uint8)opcode1;
 
-                if (frame_ip < frame_ip_end) {
-                    opcode = *frame_ip++;
-                }
                 if (opcode != WASM_OP_ATOMIC_FENCE) {
                     read_leb_uint32(frame_ip, frame_ip_end, align);
                     read_leb_uint32(frame_ip, frame_ip_end, offset);

diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c
@@ -3511,6 +3511,8 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                 uint32 opcode1;
 
                 read_leb_uint32(frame_ip, frame_ip_end, opcode1);
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
                 opcode = (uint8)opcode1;
 
                 switch (opcode) {
@@ -3843,8 +3845,12 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
             HANDLE_OP(WASM_OP_ATOMIC_PREFIX)
             {
                 uint32 offset = 0, align, addr;
+                uint32 opcode1;
 
-                opcode = *frame_ip++;
+                read_leb_uint32(frame_ip, frame_ip_end, opcode1);
+                /* opcode1 was checked in loader and is no larger than
+                   UINT8_MAX */
+                opcode = (uint8)opcode1;
 
                 if (opcode != WASM_OP_ATOMIC_FENCE) {
                     read_leb_uint32(frame_ip, frame_ip_end, align);

diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c
@@ -5092,9 +5092,13 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache,
 #if (WASM_ENABLE_WAMR_COMPILER != 0) || (WASM_ENABLE_JIT != 0)
             case WASM_OP_SIMD_PREFIX:
             {
-                /* TODO: shall we ceate a table to be friendly to branch
-                 * prediction */
-                opcode = read_uint8(p);
+                uint32 opcode1;
+
+                read_leb_uint32(p, p_end, opcode1);
+                /* opcode1 was checked in wasm_loader_prepare_bytecode and
+                   is no larger than UINT8_MAX */
+                opcode = (uint8)opcode1;
+
                 /* follow the order of enum WASMSimdEXTOpcode in wasm_opcode.h
                  */
                 switch (opcode) {
@@ -5184,8 +5188,14 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache,
 #if WASM_ENABLE_SHARED_MEMORY != 0
             case WASM_OP_ATOMIC_PREFIX:
             {
-                /* atomic_op (1 u8) + memarg (2 u32_leb) */
-                opcode = read_uint8(p);
+                uint32 opcode1;
+
+                /* atomic_op (u32_leb) + memarg (2 u32_leb) */
+                read_leb_uint32(p, p_end, opcode1);
+                /* opcode1 was checked in wasm_loader_prepare_bytecode and
+                   is no larger than UINT8_MAX */
+                opcode = (uint8)opcode1;
+
                 if (opcode != WASM_OP_ATOMIC_FENCE) {
                     skip_leb_uint32(p, p_end); /* align */
                     skip_leb_uint32(p, p_end); /* offset */
@@ -9836,8 +9846,8 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
             {
                 uint32 opcode1;
 
-                CHECK_BUF(p, p_end, 1);
-                opcode1 = read_uint8(p);
+                read_leb_uint32(p, p_end, opcode1);
+
                 /* follow the order of enum WASMSimdEXTOpcode in wasm_opcode.h
                  */
                 switch (opcode1) {
@@ -10498,8 +10508,8 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
             {
                 uint32 opcode1;
 
-                CHECK_BUF(p, p_end, 1);
-                opcode1 = read_uint8(p);
+                read_leb_uint32(p, p_end, opcode1);
+
 #if WASM_ENABLE_FAST_INTERP != 0
                 emit_byte(loader_ctx, opcode1);
 #endif

diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c
@@ -3492,8 +3492,11 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache,
                 uint32 opcode1;
 
                 read_leb_uint32(p, p_end, opcode1);
+                /* opcode1 was checked in wasm_loader_prepare_bytecode and
+                   is no larger than UINT8_MAX */
+                opcode = (uint8)opcode1;
 
-                switch (opcode1) {
+                switch (opcode) {
                     case WASM_OP_I32_TRUNC_SAT_S_F32:
                     case WASM_OP_I32_TRUNC_SAT_U_F32:
                     case WASM_OP_I32_TRUNC_SAT_S_F64:
@@ -3549,8 +3552,14 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache,
 #if WASM_ENABLE_SHARED_MEMORY != 0
             case WASM_OP_ATOMIC_PREFIX:
             {
-                /* atomic_op (1 u8) + memarg (2 u32_leb) */
-                opcode = read_uint8(p);
+                uint32 opcode1;
+
+                /* atomic_op (u32_leb) + memarg (2 u32_leb) */
+                read_leb_uint32(p, p_end, opcode1);
+                /* opcode1 was checked in wasm_loader_prepare_bytecode and
+                   is no larger than UINT8_MAX */
+                opcode = (uint8)opcode1;
+
                 if (opcode != WASM_OP_ATOMIC_FENCE) {
                     skip_leb_uint32(p, p_end); /* align */
                     skip_leb_uint32(p, p_end); /* offset */
@@ -7464,11 +7473,14 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
 #if WASM_ENABLE_SHARED_MEMORY != 0
             case WASM_OP_ATOMIC_PREFIX:
             {
-                opcode = read_uint8(p);
+                uint32 opcode1;
+
+                read_leb_uint32(p, p_end, opcode1);
+
 #if WASM_ENABLE_FAST_INTERP != 0
-                emit_byte(loader_ctx, opcode);
+                emit_byte(loader_ctx, opcode1);
 #endif
-                if (opcode != WASM_OP_ATOMIC_FENCE) {
+                if (opcode1 != WASM_OP_ATOMIC_FENCE) {
                     CHECK_MEMORY();
                     read_leb_uint32(p, p_end, align);      /* align */
                     read_leb_uint32(p, p_end, mem_offset); /* offset */
@@ -7479,7 +7491,7 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
 #if WASM_ENABLE_JIT != 0 || WASM_ENABLE_WAMR_COMPILER != 0
                 func->has_memory_operations = true;
 #endif
-                switch (opcode) {
+                switch (opcode1) {
                     case WASM_OP_ATOMIC_NOTIFY:
                         POP2_AND_PUSH(VALUE_TYPE_I32, VALUE_TYPE_I32);
                         break;

diff --git a/wamr-compiler/main.c b/wamr-compiler/main.c
@@ -627,8 +627,10 @@ main(int argc, char *argv[])
             goto fail1;
     }
 
-    if (get_package_type(wasm_file, wasm_file_size) != Wasm_Module_Bytecode) {
-        printf("Invalid file type: expected wasm file but got other\n");
+    if (wasm_file_size >= 4 /* length of MAGIC NUMBER */
+        && get_package_type(wasm_file, wasm_file_size)
+               != Wasm_Module_Bytecode) {
+        printf("Invalid wasm file: magic header not detected\n");
         goto fail2;
     }