Fixes potential issue in length handling in array.new (Wasm GC)#4943
Fixes potential issue in length handling in array.new (Wasm GC)#4943srberard wants to merge 1 commit into
array.new (Wasm GC)#4943Conversation
Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
srberard
requested review from
TianlongLiang,
lum1n0us,
no1wudi and
yamt
as code owners
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #4942 by hardening array.new length handling in the interpreter loader’s GC-enabled constant-expression parsing (load_init_expr) to avoid integer-overflow-driven undersized allocations that can lead to heap buffer overflows during module loading.
Changes:
- Adjusts allocation size arithmetic for
WASM_OP_ARRAY_NEWto avoidint32→uint64sign-extension overflow by casting the length throughuint32. - Treats
array.new_default’s length asuint32when storing it into the init-expr value.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| size = | ||
| sizeof(WASMArrayNewInitValues) | ||
| + sizeof(WASMValue) * (uint64)len_val.i32; | ||
| + sizeof(WASMValue) | ||
| * (uint64)(uint32)len_val.i32; | ||
| if (!(array_init_values = loader_malloc( |
| size = | ||
| sizeof(WASMArrayNewInitValues) | ||
| + sizeof(WASMValue) * (uint64)len_val.i32; | ||
| + sizeof(WASMValue) | ||
| * (uint64)(uint32)len_val.i32; |
lum1n0us
left a comment
lum1n0us
left a comment
There was a problem hiding this comment.
Thanks for the fix. The signed/unsigned overflow issue appears to be resolved.
One remaining concern is that array.new still allocates based on len_val, so excessively large lengths may still cause unnecessary memory pressure in the loader. That seems worth addressing, but I’m also okay with treating it as a separate follow-up if it is outside the intended scope of this PR.
Fixes #4942