ref-fvm icon indicating copy to clipboard operation
ref-fvm copied to clipboard

Wasm Validation Rules

Open Stebalien opened this issue 2 years ago • 15 comments

  • Can only export an invoke(i32) -> i32 function. This will likely change to allow a validate and/or constructor function in the near future, but we're not allowing arbitrary functions.
  • Cannot export any globals? We don't currently enforce this, but modules have no reason to do so.
  • Cannot import any globals.
  • Can only import functions defined in the syscall API (https://github.com/filecoin-project/ref-fvm/blob/a8f15212940faa3074848605a19cd9ede286e1ef/fvm/src/syscalls/mod.rs#L97).
  • Cannot use reference types.
  • Cannot import or export any tables (I think?).
  • Cannot import any memories.
  • Must export exactly one memory ("memory"). https://github.com/filecoin-project/ref-fvm/blob/23d4ed325c6bd348aebab4c0bc9a86f39bd41675/fvm/src/syscalls/mod.rs#L139-L142
  • Cannot use atomics.

Stebalien avatar Oct 28 '22 15:10 Stebalien

cc @Jacarte

Stebalien avatar Oct 28 '22 15:10 Stebalien

Adding another rule:

  • The binary should not exceed 5Mb in size (the instrumentation/compilation is taking a remarkably amount of time now)

Jacarte avatar Nov 09 '22 17:11 Jacarte

Allocates at most X MiB of memory up-front (i.e., doesn't statically declare more than some amount of minimum memory).

Stebalien avatar Nov 11 '22 19:11 Stebalien

Currently, code can run before invoke, but it'll run out of gas immediately and trap (leading to a fatal error).

We need to either:

  1. Properly account for this kind of code.
  2. Forbid it.

Ideally, we'd just forbid it. It looks like we can run wasm-ctor-eval (from binaryen)?

Specifically, we need to handle:

  1. The start function.
  2. Table initialization functions. See the exprs that appear in https://webassembly.github.io/spec/core/syntax/modules.html#syntax-elem.

Migrated from https://github.com/filecoin-project/ref-fvm/issues/607.

Stebalien avatar Nov 12 '22 00:11 Stebalien

Another rule to add:

  • Limit the initial memory

Apparently, cranelift is packaging the initial declared memory in the module. A simple Wasm binary of 600bytes can go to several Mb in the compiled obj.

Jacarte avatar Nov 30 '22 18:11 Jacarte

I wonder if it's related to https://docs.wasmtime.dev/api/wasmtime/struct.Config.html#method.memory_init_cow?

Stebalien avatar Nov 30 '22 19:11 Stebalien

Yeah, it looks like https://docs.wasmtime.dev/api/wasmtime/struct.Config.html#method.memory_guaranteed_dense_image_size. Which sets a maximum of 16MiB, which isn't terrible but something we may want to lower.

Stebalien avatar Nov 30 '22 19:11 Stebalien

@aakoshh any reason to leave memory_init_cow enabled? IMO, we should consider disabling it because we'll need to charge for it anyways.

Stebalien avatar Nov 30 '22 19:11 Stebalien

@aakoshh any reason to leave memory_init_cow enabled? IMO, we should consider disabling it because we'll need to charge for it anyways.

Can it be related to this https://github.com/advisories/GHSA-wh6w-3828-g9qf ?

Jacarte avatar Dec 02 '22 16:12 Jacarte

BTW, https://github.com/advisories/GHSA-44mr-8vmm-wjhg and https://github.com/advisories/GHSA-wh6w-3828-g9qf are two security CVEs related to wasmtime version 1.0.2 used in the ref-fvm. We should start thinking on migrating.

Jacarte avatar Dec 02 '22 16:12 Jacarte

Maybe I'm blind, but it looks to me like they zeroed out any non-empty image, if the next image was None: https://github.com/bytecodealliance/wasmtime/blob/v1.0.2/crates/runtime/src/cow.rs#L387

I don't see how the scenarios described in https://github.com/advisories/GHSA-wh6w-3828-g9qf can happen :thinking:

aakoshh avatar Dec 05 '22 20:12 aakoshh

Maybe I'm blind, but it looks to me like they zeroed out any non-empty image, if the next image was None: https://github.com/bytecodealliance/wasmtime/blob/v1.0.2/crates/runtime/src/cow.rs#L387

I don't see how the scenarios described in GHSA-wh6w-3828-g9qf can happen 🤔

But the limits of the zeroed memory changes from one to other instantiation, right ? https://github.com/bytecodealliance/wasmtime/blob/15991947f485a60298e45d1d915125c0f3cc5a8a/crates/runtime/src/cow.rs#L390

I only see the issue if we reuse the same engine between actor initializations/calls.

(Disclaimer here, I am also struggling to see how this scenario can happen here :) )

Jacarte avatar Dec 06 '22 08:12 Jacarte

We also need to account for size in the wasmtime pooling instance config. Specifically, We'll need to make sure we can fit the maximum number of imports/exports/functions/types/globals/etc. in the maximum instance size.

Stebalien avatar Dec 21 '22 01:12 Stebalien

Currently, code can run before invoke, but it'll run out of gas immediately and trap (leading to a fatal error).

We need to either:

1. Properly account for this kind of code.

2. Forbid it.

Ideally, we'd just forbid it. It looks like we can run wasm-ctor-eval (from binaryen)?

Specifically, we need to handle:

1. The start function.

2. Table initialization functions. See the `expr`s that appear in https://webassembly.github.io/spec/core/syntax/modules.html#syntax-elem.

Migrated from #607.

It seems we can use wizer here, https://github.com/bytecodealliance/wizer

Jacarte avatar Jul 31 '23 14:07 Jacarte

Addresssing MVP in https://github.com/filecoin-project/Fuzzing-FVM/pull/776

Jacarte avatar Jul 31 '23 16:07 Jacarte