Potential Issue with Flags Defined as uint32 Instead of uint8 in WAMR

[epitaphial]

During the wasm module loading phase, for instance in the load_memory_import function, the mem_flag is defined as a uint32 rather than a uint8. Similarly, in the load_table_import function, the table_flag is also defined as a uint32. However, according to the WebAssembly Memory64 binary limits, these flag values can be represented in just one byte. If they are defined as a uint32 and read using read_leb_uint32, the first parameter of the wasm_table_check_flags function (which is declared as const uint8 table_flag) will have its value truncated to a uint8. This behavior may cause an issue where an invalid flag value that exceeds one byte will not be properly detected by the wasm_table_check_flags function.

[TianlongLiang]

If read_leb_uint32 loads a memory flag larger than one byte, the following code will detect it:

    if (p - p_org > 1) {
        LOG_VERBOSE("integer representation too long(memory)");
        set_error_buf(error_buf, error_buf_size, "invalid limits flags");
        return false;
    }

The same goes for table flags, so it won't be any problem

[epitaphial]

If read_leb_uint32 loads a memory flag larger than one byte, the following code will detect it:

if (p - p_org > 1) {
    LOG_VERBOSE("integer representation too long(memory)");
    set_error_buf(error_buf, error_buf_size, "invalid limits flags");
    return false;
}

The same goes for table flags, so it won't be any problem

Yes, but in the file wasm_mini_loader.c, after the functions load_memory_import and load_table_import call read_leb_uint32 to read the flag, there is no check for the flag's length. I suspect that the corresponding implementation might have been omitted.

[TianlongLiang]

You are right about those two places, I will add the assert check.

PS: The miniloader has some strong assumptions about the WASM file format, so it can minimize the time and code size needed for the loading phase. It aims to load the correct WASM file correctly. The bh_assert will only be effective in debug builds.

[lum1n0us]

just FYI: the main design objective or the essential requirement for the mini-loader is that it contains no checker and always assumes that the input provided to the mini-loader is valid.