Skip to content

Comments

Add missing array bounds checks in visitArrayRMW and visitArrayCmpxchg#8351

Merged
kripken merged 1 commit intoWebAssembly:mainfrom
sumleo:fix-array-rmw-bounds-check
Feb 21, 2026
Merged

Add missing array bounds checks in visitArrayRMW and visitArrayCmpxchg#8351
kripken merged 1 commit intoWebAssembly:mainfrom
sumleo:fix-array-rmw-bounds-check

Conversation

@sumleo
Copy link
Contributor

@sumleo sumleo commented Feb 20, 2026

Summary

The interpreter's visitArrayRMW and visitArrayCmpxchg in wasm-interpreter.h access data->values[indexVal] without verifying that indexVal is within the array's bounds. This can lead to out-of-bounds memory access when the index exceeds the array length.

This PR adds the same bounds check pattern already used by visitArrayGet and visitArraySet:

if (indexVal >= data->values.size()) {
  trap("array oob");
}

The check is added in both visitArrayRMW and visitArrayCmpxchg, immediately after computing indexVal and before accessing data->values[indexVal].

Test plan

  • All 309 existing unit tests pass (binaryen-unittests).
  • The fix follows the exact same pattern as the existing bounds checks in visitArrayGet (line 2318) and visitArraySet (line 2333), so it is consistent with the rest of the codebase.

…ayCmpxchg

The interpreter's visitArrayRMW and visitArrayCmpxchg were accessing
data->values[indexVal] without first checking if indexVal is within
bounds. This could lead to out-of-bounds memory access when the index
exceeds the array length.

Add the same bounds check pattern used by visitArrayGet and
visitArraySet: trap with "array oob" when indexVal >= data->values.size().
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Normally this would require a test, but the upstream spec tests should cover it when they are finished.

@kripken kripken merged commit c994521 into WebAssembly:main Feb 21, 2026
18 checks passed
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