SPY / haskell-wasm

Haskell WebAssembly Toolkit
Other
151 stars 24 forks source link

Allowed out-of-bounds memory access #14

Open agustinmista opened 3 years ago

agustinmista commented 3 years ago

Hi @SPY!

I think I found another bug where it is possible to store values on out-of-bounds memory addresses:

ghci> m
Module {types = [FuncType {params = [I32,F32], results = [I32]}], functions = [Function {funcType = 0, localTypes = [], body = [I64Const 0,F32Const (-1.1170084),ITruncFS BS32 BS32,GetLocal 0,I32Store (MemArg {offset = 1, align = 2}),I32Const 2,Return]}], tables = [], mems = [Memory (Limit 1 Nothing)], globals = [], elems = [], datas = [], start = Nothing, imports = [], exports = [Export {name = "foo", desc = ExportFunc 0}]}

The reference implementation rejects this with a runtime trap:

$ ./wasm foo.wasm -e '(invoke "foo" (i32.const 2) (f32.const 0))'
foo.wasm:0x3e: runtime trap: out of bounds memory access

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" [VI32 2, VF32 0]
Just [VI32 2]

I'm not sure whether the issue comes from the I32Store instruction, or from the ITruncFS one. I will try to dig further to see what could be happening, but I would appreciate it if you can confirm that the issue is reproducible on your side.

Thanks for the great support! I will send you PRs for all the bugs I can find after I'm done with the testing part 😄

/Agustín

SPY commented 3 years ago

Hi Agustin

A nice catch as usual :) An issue now arises from Int32 overflow. Truncate puts 0xFFFFFFFF on stack to be used as an address. But I32Store instruction also has offset field equals to 1. Here we got 0xFFFFFFFF + 1 evaluated to 0, but boundary check is made later. It should be easy to fix by replacing

let addr = fromIntegral $ va + fromIntegral offset

with

let addr = fromIntegral $ fromIntegral va + offset