Browse Source

Fix few integer overflowing (#4161)

- fix(interpreter): correct offset calculations in wasm_loader_get_const_offset function
- fix(mem-alloc): update offset calculation in gc_migrate for memory migration
- add pointer-overflow sanitizer
liang.he 9 months ago
parent
commit
793135b41c

+ 3 - 0
build-scripts/config_common.cmake

@@ -157,6 +157,9 @@ elseif (WAMR_BUILD_SANITIZER STREQUAL "asan")
 elseif (WAMR_BUILD_SANITIZER STREQUAL "tsan")
   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -O0 -fno-omit-frame-pointer -fsanitize=thread -fno-sanitize-recover=all" )
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=thread")
+elseif (WAMR_BUILD_SANITIZER STREQUAL "posan")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -O0 -fno-omit-frame-pointer -fsanitize=pointer-overflow -fno-sanitize-recover=all" )
+  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=pointer-overflow")
 elseif (NOT (WAMR_BUILD_SANITIZER STREQUAL "") )
   message(SEND_ERROR "Unsupported sanitizer: ${WAMR_BUILD_SANITIZER}")
 endif()

+ 13 - 6
core/iwasm/interpreter/wasm_loader.c

@@ -9693,8 +9693,10 @@ wasm_loader_get_const_offset(WASMLoaderContext *ctx, uint8 type, void *value,
                 *offset = 0;
                 return true;
             }
-            *offset = -(uint32)(ctx->i64_const_num * 2 + ctx->i32_const_num)
-                      + (uint32)(i64_const - ctx->i64_consts) * 2;
+
+            /* constant index is encoded as negative value */
+            *offset = -(int32)(ctx->i64_const_num * 2 + ctx->i32_const_num)
+                      + (int32)(i64_const - ctx->i64_consts) * 2;
         }
         else if (type == VALUE_TYPE_V128) {
             V128 key = *(V128 *)value, *v128_const;
@@ -9704,9 +9706,12 @@ wasm_loader_get_const_offset(WASMLoaderContext *ctx, uint8 type, void *value,
                 *offset = 0;
                 return true;
             }
-            *offset = -(uint32)(ctx->v128_const_num)
-                      + (uint32)(v128_const - ctx->v128_consts);
+
+            /* constant index is encoded as negative value */
+            *offset = -(int32)(ctx->v128_const_num)
+                      + (int32)(v128_const - ctx->v128_consts);
         }
+
         else {
             int32 key = *(int32 *)value, *i32_const;
             i32_const = bsearch(&key, ctx->i32_consts, ctx->i32_const_num,
@@ -9715,8 +9720,10 @@ wasm_loader_get_const_offset(WASMLoaderContext *ctx, uint8 type, void *value,
                 *offset = 0;
                 return true;
             }
-            *offset = -(uint32)(ctx->i32_const_num)
-                      + (uint32)(i32_const - ctx->i32_consts);
+
+            /* constant index is encoded as negative value */
+            *offset = -(int32)(ctx->i32_const_num)
+                      + (int32)(i32_const - ctx->i32_consts);
         }
 
         return true;

+ 22 - 2
core/shared/mem-alloc/ems/ems_kfc.c

@@ -208,8 +208,28 @@ gc_get_heap_struct_size()
 static void
 adjust_ptr(uint8 **p_ptr, intptr_t offset)
 {
-    if (*p_ptr)
-        *p_ptr = (uint8 *)((intptr_t)(*p_ptr) + offset);
+    if ((!*p_ptr)) {
+        return;
+    }
+
+    /*
+     * to resolve a possible signed integer overflow issue
+     * when p_ptr is over 0x8000000000000000  by not using
+     * `(intptr_t)`
+     */
+    uintptr_t offset_val = 0;
+#if UINTPTR_MAX == UINT64_MAX
+    offset_val = labs(offset);
+#else
+    offset_val = abs(offset);
+#endif
+
+    if (offset > 0) {
+        *p_ptr = (uint8 *)((uintptr_t)(*p_ptr) + offset_val);
+    }
+    else {
+        *p_ptr = (uint8 *)((uintptr_t)(*p_ptr) - offset_val);
+    }
 }
 
 int

+ 4 - 2
tests/wamr-test-suites/spec-test-script/runtest.py

@@ -1575,7 +1575,8 @@ if __name__ == "__main__":
         try:
             if not opts.no_cleanup:
                 # remove the files under /tempfiles/ and copy of .wasm files
-                log(f"Removing {temp_file_repo}")
+                log(f"Removing tmp*")
+                # log(f"Removing {temp_file_repo}")
 
                 for t in temp_file_repo:
                     # None and empty
@@ -1585,7 +1586,8 @@ if __name__ == "__main__":
                     if os.path.exists(t):
                         os.remove(t)
             else:
-                log(f"Leaving {temp_file_repo}")
+                log(f"Leaving tmp*")
+                # log(f"Leaving {temp_file_repo}")
             
         except Exception as e:
             print("Failed to remove tempfiles: %s" % e)

+ 6 - 1
tests/wamr-test-suites/test_wamr.sh

@@ -39,7 +39,7 @@ function help()
     echo "-F set the firmware path used by qemu"
     echo "-C enable code coverage collect"
     echo "-j set the platform to test"
-    echo "-T set sanitizer to use in tests(ubsan|tsan|asan)"
+    echo "-T set sanitizer to use in tests(ubsan|tsan|asan|posan)"
     echo "-A use the specified wamrc command instead of building it"
     echo "-r [requirement name] [N [N ...]] specify a requirement name followed by one or more"
     echo "                                  subrequirement IDs, if no subrequirement is specificed,"
@@ -1035,6 +1035,11 @@ function trigger()
         EXTRA_COMPILE_FLAGS+=" -DWAMR_BUILD_SANITIZER=tsan"
     fi
 
+    if [[ "$WAMR_BUILD_SANITIZER" == "posan" ]]; then
+        echo "Setting run with posan"
+        EXTRA_COMPILE_FLAGS+=" -DWAMR_BUILD_SANITIZER=posan"
+    fi
+
     # Make sure we're using the builtin WASI libc implementation
     # if we're running the wasi certification tests.
     if [[ $TEST_CASE_ARR ]]; then