Fix wrong condition in Memory64 data segment loading#4848
Open
sumleo wants to merge 2 commits intobytecodealliance:mainfrom
Open
Fix wrong condition in Memory64 data segment loading#4848sumleo wants to merge 2 commits intobytecodealliance:mainfrom
sumleo wants to merge 2 commits intobytecodealliance:mainfrom
Conversation
The condition for selecting between imported and defined memories uses `import_memory_count > 0` when it should use `mem_index < import_memory_count`. When a module has both imported and defined memories and a data segment targets a defined memory (mem_index >= import_memory_count), the current condition incorrectly reads from the import_memories array out of bounds.
sumleo
requested review from
TianlongLiang,
lum1n0us,
no1wudi and
yamt
as code owners
core/iwasm/interpreter/wasm_loader.c
Outdated
| /* This memory_flag is from memory instead of data segment */ | ||
| uint8 memory_flag; | ||
| if (module->import_memory_count > 0) { | ||
| if (mem_index < module->import_memory_count) { |
Contributor
There was a problem hiding this comment.
better refactor this code snippet with has_module_memory64 instead, and modify the wasm_mini_loder.c too
Replace the manual import/defined memory flag lookup with a call to the existing has_module_memory64() helper, which already encapsulates this logic correctly. Apply the same change to wasm_mini_loader.c.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In
load_data_segment_section, the Memory64 code path manually inspectsimport/defined memory arrays to determine the memory64 flag. The condition
module->import_memory_count > 0is incorrect when a module has both importedand defined memories — a data segment targeting a defined memory
(
mem_index >= import_memory_count) would still enter the import branch,causing an out-of-bounds read.
Fix
Replace the manual flag lookup with the existing
has_module_memory64()helper,which already encapsulates this logic correctly. Apply the same refactor to both
wasm_loader.candwasm_mini_loader.c.Before:
After:
Impact
Fixes an out-of-bounds array access when a module with both imported and defined
memories has a data segment targeting a defined memory with Memory64 enabled.