Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

WASM: Disable internal memory #7842

Merged
merged 1 commit into from
Feb 12, 2018
Merged

WASM: Disable internal memory #7842

merged 1 commit into from
Feb 12, 2018

Conversation

NikVolf
Copy link
Contributor

@NikVolf NikVolf commented Feb 8, 2018

It's useless for meaningful contracts, while having it opens network for dos attack

@NikVolf NikVolf added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. B0-patch labels Feb 8, 2018
@5chdn 5chdn added this to the 1.10 milestone Feb 8, 2018
@rphmeier
Copy link
Contributor

rphmeier commented Feb 8, 2018

if it's metered the same way as the regular (?) memory is it really a DoS vector?

@NikVolf
Copy link
Contributor Author

NikVolf commented Feb 8, 2018

@rphmeier sure, could be, but webassembly spec does not allow to judge about module internal memory (as long as it is not exported) during execution

while we of course can hack around and meter internal memoery it by the module composition before instantiation, I would prefer to avoid it at all so other implementations that use different engines and api-s (which might not allow this hack) could match rules

So I can still add metering, but it's really hard to imagine contract that does not use shared memory with runtime, it can do almost nothing.

@NikVolf NikVolf added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 8, 2018
@NikVolf
Copy link
Contributor Author

NikVolf commented Feb 9, 2018

@rphmeier
tl;dr; By allowing internal memory and metering it we gain almost nothing while impeding compatibility with other implementations

@NikVolf NikVolf added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Feb 9, 2018
@5chdn 5chdn merged commit fab0339 into master Feb 12, 2018
@5chdn 5chdn deleted the check-mem branch February 12, 2018 10:19
tomusdrw pushed a commit that referenced this pull request Feb 14, 2018
5chdn pushed a commit that referenced this pull request Feb 14, 2018
* update back-references more aggressively after answering from cache (#7578)

* Add new EF ropstens nodes. (#7824)

* Add new EF ropstens nodes.

* Fix tests

* Add a timeout for light client sync requests (#7848)

* Add a timeout for light client sync requests

* Adjusting timeout to number of headers

* Flush keyfiles. Resolves #7632 (#7868)

* Fix wallet import (#7873)

* rpc: generate new account id for imported wallets

* ethstore: handle duplicate wallet filenames

* ethstore: simplify deduplication of wallet file names

* ethstore: do not dedup wallet filenames on update

* ethstore: fix minor grumbles

* [WASM] mem_cmp added to the Wasm runtime (#7539)

* mem_cmp added to the Wasm runtime

* schedule.wasm.mem_copy to schedule.wasm.mem_cmp for mem_cmp

* [Wasm] memcmp fix and test added (#7590)

* [Wasm] memcmp fix and test added

* [Wasm] use reqrep_test! macro for memcmp test

* wasmi interpreter (#7796)

* adjust storage update evm-style (#7812)

* disable internal memory (#7842)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants