go-interpreter / wagon

wagon, a WebAssembly-based Go interpreter, for Go.
BSD 3-Clause "New" or "Revised" License
903 stars 148 forks source link

Denial of Service via Arbitrary Allocation #147

Closed laizy closed 5 years ago

laizy commented 5 years ago

There are many places containing codes pattern: read length then allocate array or map, like:

count, err := leb128.ReadVarUint32(r)
if err != nil {
    return err
}
s.Bodies = make([]FunctionBody, count)

So the attacker can construct a invalid wasm file, set the count to 2^32-1, causing the Wagon interpreter to allocate more memory than is available and irrecoverably crash the entire process. We may need to change it to something like:

initialCap := min(count, maxInitialCap)
s.Bodies = make([]FunctionBody, 0, initialCap)
....
s.Bodies = append(s.Bodies, body)
sbinet commented 5 years ago

good point.

perhaps a helper function like this?

func readCount(r io.Reader, cap int) (int, error) { ... }
laizy commented 5 years ago

Since the Reader is a stream, so theoretically it could be very large, until we read it, if encountering eof before we read out count values, then we can say it is invalid. Although adding a max size limit works, but it is cumbersome, since different use cases may have different value, it will end up with a lot of limit parameters the user need to fill in (like max function count, max string size etc). The problem is not caused by the count, but the call to make([]Type, count), we just need to replace that from:

arr := make([]Value, count)
for i:=0; i< count; i++ {
    val := read and decode Value
   arr[i] = val
}

to :

var arr []Value // or initialCap := min(count, maxInitialCap); arr := make([]Value, 0, initialCap) to avoid some reallocation 
for i:=0; i< count; i++ {
    val := read and decode Value // if the count is too large we will encounter eof here finally.
   arr = append(arr, val)
}