Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A risk of panic in vm by using contract exported memory function #466

Closed
KamiD opened this issue Jul 9, 2020 · 4 comments
Closed

A risk of panic in vm by using contract exported memory function #466

KamiD opened this issue Jul 9, 2020 · 4 comments
Milestone

Comments

@KamiD
Copy link

KamiD commented Jul 9, 2020

Hi there, I found that cosmwasm are using allocate and deallocate by contract exported function
in use case of call_raw , we are using it like this:

let region_ptr = instance.allocate(arg.len())?;
instance.write_memory(region_ptr, arg)?;

in function write_region, I saw that we are using len and capacity to check memory size to make sure that ptr have enough space to save it
The question is: allocate and deallocate are defined in std and exported by contract, that means contract writer can do any change of allocate function as they want, if someone change the return value of allocate, faked a right len and capacity but set ptr to a broken address(or smaller capacity), is cosmwasm still running correctly?

@webmaster128
Copy link
Member

allocate and deallocate are defined in std and exported by contract, that means contract writer can do any change of allocate function as they want

Correct

if someone change the return value of allocate, faked a right len and capacity but set ptr to a broken address(or smaller capacity

Right. We need to assume that contracts return broken Regions. This must not break the VM but just the execution of one contract.

When it comes to panics, it is very important to distinguish where they happen. In a contract they are no problem. In the VM, there should not be a panic that can be troggered by the contract. The code you are referring to changes a lot betwen 0.8 and 0.9 (see packages/vm/src/memory.rs in 0.8...v0.9.3). So I assume this issue cannot occur anymore.

@webmaster128
Copy link
Member

See also #419. This is not a full protection but at least guides developers of standard libraries when their Region creation is not correct.

@KamiD
Copy link
Author

KamiD commented Jul 10, 2020

See also #419. This is not a full protection but at least guides developers of standard libraries when their Region creation is not correct.

Cool, I will test it in new version

@ethanfrey ethanfrey added this to the 0.11.0 milestone Sep 27, 2020
@webmaster128
Copy link
Member

In recent versions of CosmWasm (I just checked 0.10.1), all functions in packages/vm/src/memory.rs do not panic. Instead, errors are returned and handled properly. Feel free to re-open if I missed something.

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

No branches or pull requests

3 participants