gorocksdb icon indicating copy to clipboard operation
gorocksdb copied to clipboard

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

Open odeke-em opened this issue 2 years ago • 1 comments

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

odeke-em avatar Jul 02 '22 14:07 odeke-em

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

anilcse avatar Jul 03 '22 00:07 anilcse