tecbot / gorocksdb

gorocksdb is a Go wrapper for RocksDB
http://rocksdb.org
MIT License
937 stars 269 forks source link

(*WriteBatchIterator).decodeVarint unfortunately doesn’t correctly detect invalid varint sequences and can consume the entire buffer #221

Open odeke-em opened 2 years ago

odeke-em commented 2 years ago

Courtesy of the Cosmos Network Security, I discovered this code while auditing supply chain dependencies. If we examine this code in https://github.com/tecbot/gorocksdb/blob/37ba422d5c751e427bef06cd17fb6d3e69818240/write_batch.go#L265-L283

and then extract it and run the following https://go.dev/play/p/MXpanwJKofY or inlined below:

package main

import (
    "errors"
    "fmt"
    "io"
)

type wbi struct {
    data []byte
    err  error
}

func (iter *wbi) decodeVarint() uint64 {
    var n int
    var x uint64
    for shift := uint(0); shift < 64 && n < len(iter.data); shift += 7 {
        b := uint64(iter.data[n])
        n++
        x |= (b & 0x7F) << shift
        if (b & 0x80) == 0 {
            iter.data = iter.data[n:]
            return x
        }
    }
    if n == len(iter.data) {
        iter.err = io.ErrShortBuffer
    } else {
        iter.err = errors.New("malformed varint")
    }
    return 0
}

func main() {
    // The 10th byte here is invalid and should be reported.
    // Please see https://github.com/golang/go/issues/41185
    b := []byte{0xd7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}
    wb := &wbi{data: b}
    v := wb.decodeVarint()
    println(v)
    fmt.Printf("%v\n", wb.err) // This should have reported an error.
}

which prints

18446744073709551575
<nil>

Fix

The fix entails just using the Go standard library's code in which I fixed this bug in March 2021 in the Go standard library per https://github.com/golang/go/commit/aafad20b617ee63d58fcd4f6e0d98fe27760678c

with a demo at https://go.dev/play/p/kYGBjzh24Qx which should properly print malformed varint

package main

import (
    "encoding/binary"
    "errors"
    "fmt"
    "io"
)

type wbi struct {
    data []byte
    err  error
}

func (iter *wbi) decodeVarint() uint64 {
    var n int
    var x uint64
    for shift := uint(0); shift < 64 && n < len(iter.data); shift += 7 {
        b := uint64(iter.data[n])
        n++
        x |= (b & 0x7F) << shift
        if (b & 0x80) == 0 {
            iter.data = iter.data[n:]
            return x
        }
    }
    if n == len(iter.data) {
        iter.err = io.ErrShortBuffer
    } else {
        iter.err = errors.New("malformed varint")
    }
    return 0
}

func (iter *wbi) decodeVarint_good() (x uint64) {
    v, n := binary.Varint(iter.data)
    if n == len(iter.data) {
        iter.err = io.ErrShortBuffer
    } else if n < 0 {
        iter.err = errors.New("malformed varint")
    } else {
        x = uint64(v)
    }
    return x
}

func main() {
    // The 10th byte here is invalid and should be reported.
    // Please see https://github.com/golang/go/issues/41185
    b := []byte{0xd7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}
    wb := &wbi{data: b}
    v := wb.decodeVarint_good()
    println(v)
    fmt.Printf("%v\n", wb.err) // This should have reported an error.
}

which correctly prints

0
malformed varint
anilcse commented 2 years ago

Not sure if the repo is still active, last commit was in 2019