Skip to content

Comments

Fix missing IS_INVALID_TAGINDEX check in RETHROW handler#4837

Open
sumleo wants to merge 1 commit intobytecodealliance:mainfrom
sumleo:fix/rethrow-tag-index-validation
Open

Fix missing IS_INVALID_TAGINDEX check in RETHROW handler#4837
sumleo wants to merge 1 commit intobytecodealliance:mainfrom
sumleo:fix/rethrow-tag-index-validation

Conversation

@sumleo
Copy link
Contributor

@sumleo sumleo commented Feb 12, 2026

The RETHROW opcode handler reads exception_tag_index from the stack and directly accesses module->module->tags[exception_tag_index] without checking for INVALID_TAGINDEX.

When CATCH_ALL catches a cross-module exception with an unknown tag, it pushes INVALID_TAGINDEX (0xFFFFFFFF) onto the stack. If RETHROW then executes, it accesses tags[0xFFFFFFFF] — a massive out-of-bounds read.

The THROW handler correctly checks IS_INVALID_TAGINDEX before the array access. This patch adds the same check to the RETHROW handler: when the tag index is invalid, skip the tags[] access and set cell_num_to_copy to 0, allowing the exception to propagate to CATCH_ALL handlers.

@sumleo sumleo force-pushed the fix/rethrow-tag-index-validation branch from 4de4f92 to bb89991 Compare February 13, 2026 14:39
wasm_module_inst_t inst = wasm_runtime_instantiate(
module, 8192, 8192, error_buf, sizeof(error_buf));

if (inst) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have an assertion about inst.

if(inst) should be removed since else branch are not expected.

Copy link
Contributor

@lum1n0us lum1n0us left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch looks good to me.

However, the test cases might need additional work:

  • Case name: Use "exception-handling" or something similar instead of "wasm-interp."
  • CMakeLists.txt: I highly suggest following an existing case, such as the one in tests/unit/interpreter.

wasm_function_inst_t func =
wasm_runtime_lookup_function(inst, "f");
EXPECT_NE(func, nullptr) << "Function 'f' should be found";

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be an assertion about func.

if(func) should be removed.

wasm_runtime_create_exec_env(inst, 8192), func, 0, NULL);
/* The function may or may not succeed depending on the
* exact exception handling implementation details */
(void)ok;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better have a return value for compare.

and use wasm_runtime_get_exception to check if necessary.

Add validation for exception_tag_index in the RETHROW opcode handler
to prevent out-of-bounds access to module->module->tags[] when the
tag index is INVALID_TAGINDEX (0xFFFFFFFF). This matches the existing
check in the THROW handler.

When CATCH_ALL catches a cross-module exception with an unknown tag,
it pushes INVALID_TAGINDEX onto the stack. Without this check, a
subsequent RETHROW would access tags[0xFFFFFFFF].
@sumleo sumleo force-pushed the fix/rethrow-tag-index-validation branch from bb89991 to 88d5d3e Compare February 23, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants