SPY / haskell-wasm

Haskell WebAssembly Toolkit
Other
149 stars 24 forks source link

False positive validation with invalid memory access #13

Open agustinmista opened 3 years ago

agustinmista commented 3 years ago

Hi @SPY,

I found a randomly generated a module that passes validation despite having an invalid memory alignment:

ghci> m = Module {types = [FuncType {params = [], results = [I32]}], functions = [Function {funcType = 0, localTypes = [F64], body = [I32Const 0,IUnOp BS32 IClz,Block {resultType = [], body = [Loop {resultType = [], body = [Loop {resultType = [], body = [I32Const 2,GrowMemory,I64Load32U (MemArg {offset = 1, align = 3}),Drop]}]},Nop]}]}], tables = [], mems = [Memory (Limit 1 Nothing)], globals = [], elems = [], datas = [], start = Nothing, imports = [], exports = [Export {name = "foo", desc = ExportFunc 0}]}

The reference implementation rejects this module at validation time:

$ ./wasm foo.wasm
foo.wasm:0x42: invalid module: alignment must not be larger than natural

Whereas haskell-wasm accepts and runs it without complaining:

ghci> Right vm = validate m
ghci> Right (mi, s) <- instantiate emptyStore mempty vm
ghci> invokeExport s mi "foo" []
Just [VI32 32]

As far as I can tell, it looks like this should be checked when you call checkMemoryInstr for this instruction during validation, but for some reason it is not. The nested Loops look suspicious, though.

I will dig further to see if I can find the root of the issue. I would appreciate if you can confirm that you could reproduce this bug on your side.

Thanks!

/Agustín

SPY commented 3 years ago

Hi, @agustinmista

Thank you for continuing to challenge my implementation :) Again, nice catch. If you look on signed version of the same instruction you will see it has a correct size parameter 4. I think it is copy-paste typo for unsigned version. I will make a patch when I reach a machine with project setup(or you can make a pull request!).

Thanks, Ilya