Skip to content

Prevent an integer overflow when checking start_func_index#3577

Closed
lum1n0us wants to merge 1 commit intobytecodealliance:mainfrom
lum1n0us:fix/int_overflow_init_data
Closed

Prevent an integer overflow when checking start_func_index#3577
lum1n0us wants to merge 1 commit intobytecodealliance:mainfrom
lum1n0us:fix/int_overflow_init_data

Conversation

@lum1n0us
Copy link
Contributor

if (module->start_func_index != (uint32)-1
&& (module->start_func_index
>= module->import_func_count + module->func_count)) {
>= (uint64)module->import_func_count + module->func_count)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the overflow check should be done earlier, regardless of start_func_index.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, maybe we can check the integer overflow after reading module->func_count

@yamt
Copy link
Contributor

yamt commented Jun 30, 2024

btw, does it make much sense to perform fuzzing on the aot loader? (i guess it's what you are doing. this is just a guess because i have no access to the given oss-fuzz url.)
unlike wasm modules, we somehow need to trust aot modules in some extent anyway.

@lum1n0us
Copy link
Contributor Author

lum1n0us commented Jul 1, 2024

wamr in oss-fuzz

It is a problem. We're kind of hesitant to do that(fix aot related problems).

We do need to trust aot modules but also want to robust(for aot_loader) and protection(since there is no case for wamrc currently). So, we are using a "better than nothing" policy until figure it out clearly.

@lum1n0us
Copy link
Contributor Author

lum1n0us commented Jul 2, 2024

Use #3579 to fix

@lum1n0us lum1n0us closed this Jul 2, 2024
@lum1n0us lum1n0us deleted the fix/int_overflow_init_data branch November 20, 2024 05:57
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.

3 participants