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

Validation: Allow indirect calls #158

Open
JakeOShannessy opened this issue Jun 7, 2019 · 6 comments
Open

Validation: Allow indirect calls #158

JakeOShannessy opened this issue Jun 7, 2019 · 6 comments
Labels
S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. T-ewasm Target system: Ethereum Wasm

Comments

@JakeOShannessy
Copy link
Contributor

This is part of the validation process. WASM has a concept of indirect calls which we don't currently check for in the validation code. This should be just some extra logic but I'm making an issue for it so we don't forget. I want to get end-to-end tests working well before we worry about edge cases.

@JakeOShannessy JakeOShannessy self-assigned this Jun 7, 2019
@JakeOShannessy JakeOShannessy added the T-ewasm Target system: Ethereum Wasm label Jun 7, 2019
@JakeOShannessy
Copy link
Contributor Author

This is loosely analogous to to the dynamic jump/call problem we had in EVM. In summary, an indirect call is "call one of the functions in this table, here is an index to choose which function". The index is determined dynamically, so we can't know statically what the end called function will be. I think the simplest and most robust option is to ensure that dcall is not accessible from the table.

For now I have regarded all indirect calls as a risk and forbidden them. This makes the validator safe, although it will consider some safe contracts unsafe until we allow for indirect calls.

@JakeOShannessy JakeOShannessy changed the title Validation: Consider indirect calls Validation: Allow indirect calls Jun 7, 2019
@Latrasis
Copy link
Member

Latrasis commented Jun 7, 2019

Ok let's put this on hold for now.

@Latrasis Latrasis added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. label Jun 7, 2019
@JakeOShannessy
Copy link
Contributor Author

Tables can be filled one of two ways by the host (such as the parity node, we don't care about this we need to trust the node anyway) and via the element section.

If there are no elems then the contract is safe, as there is no way for dcall to be added to the table.

An elem segment is (elem $table_id $offset ($func1 $func2 ...)), where $func1 etc. are references to functions (including imports). As such, indirect calls are valid if dcall is not added to the table in this manner. Therefore:

A contract is a valid procedure iff there are no elems which reference the imported dcall.

@JakeOShannessy
Copy link
Contributor Author

Ok, I just wanted to note that information. It looks pretty straightforward to add, we just iterate though elems the same way we iterate through codes to ensure that there are no references to dcall. From what I've read it will be pretty critical. For example traits in Rust use call_indirect. The kernel has 55 call_indirects.

@Latrasis
Copy link
Member

@JakeOShannessy: Let's put this on hold for now, until preliminary issues are done. If not, we can skip it.

@JakeOShannessy
Copy link
Contributor Author

I thought it already was on hold. We won't be able to skip it in the end because nothing will pass validation if we do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. T-ewasm Target system: Ethereum Wasm
Projects
None yet
Development

No branches or pull requests

2 participants