Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
odeke-em opened this issue Jul 2, 2022 · 1 comment

Comments

@odeke-em
Copy link

odeke-em commented Jul 2, 2022

Courtesy of the Cosmos Network Security, I discovered this code while auditing supply chain dependencies.
If we examine this code in

gorocksdb/write_batch.go

Lines 265 to 283 in 37ba422

func (iter *WriteBatchIterator) 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
}

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 golang/go@aafad20

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
Copy link

anilcse commented Jul 3, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants