From 190314f8f8364790414fbef468cd380cdc96f6dc Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 21 Feb 2020 13:38:39 +0100 Subject: [PATCH 1/5] go: Refactor fuzzing support --- go/Makefile | 55 ++++--- go/common/fuzz/fuzz.go | 140 +++--------------- go/consensus/tendermint/fuzz/.gitignore | 5 - go/consensus/tendermint/fuzz/fuzz.go | 2 +- .../tendermint/fuzz/gencorpus/main.go | 32 ---- go/go.mod | 1 + go/go.sum | 2 + go/storage/fuzz/gencorpus/main.go | 40 ----- go/storage/mkvs/urkel/fuzz/.gitignore | 5 - go/storage/mkvs/urkel/fuzz/gencorpus/main.go | 30 ---- 10 files changed, 56 insertions(+), 256 deletions(-) delete mode 100644 go/consensus/tendermint/fuzz/.gitignore delete mode 100644 go/consensus/tendermint/fuzz/gencorpus/main.go delete mode 100644 go/storage/fuzz/gencorpus/main.go delete mode 100644 go/storage/mkvs/urkel/fuzz/.gitignore delete mode 100644 go/storage/mkvs/urkel/fuzz/gencorpus/main.go diff --git a/go/Makefile b/go/Makefile index 41314203e0d..65624c830c4 100644 --- a/go/Makefile +++ b/go/Makefile @@ -69,37 +69,52 @@ integrationrunner: @$(ECHO) "$(CYAN)*** Testing oasis-node with coverate...$(OFF)" @$(GO) test $(GOFLAGS) -c -covermode=atomic -coverpkg=./... -o oasis-node/$@/$@.test ./oasis-node/$@ -# Fuzzing binaries. -fuzz-targets := consensus/tendermint/fuzz/fuzz-fuzz.zip \ - storage/fuzz/fuzz-fuzz.zip \ - storage/mkvs/urkel/fuzz/fuzz-fuzz.zip +# Fuzzing. +fuzz-targets := fuzz-consensus \ + fuzz-storage \ + fuzz-mkvs/Tree \ + fuzz-mkvs/Proof -build-fuzz: $(fuzz-targets) -%/fuzz-fuzz.zip: .FORCE - @echo "Building $@" - @cd "$$(dirname "$@")"; go-fuzz-build - @cd "$$(dirname "$@")/gencorpus"; env -u GOPATH $(OASIS_GO) build -tags gofuzz - -# Run fuzzing. define canned-fuzz-run -set -x; cd "$<"; \ -if ! [ -d corpus ]; then \ - mkdir corpus; \ - pushd corpus; \ - ../gencorpus/gencorpus; \ - popd; \ +@TARGETDIR=$(shell pwd)/$<; \ +WORKDIR=/tmp/oasis-node-$@; \ +if [ "$(FUZZ_NO_BUILD)" != "1" ]; then \ + mkdir -p "$$WORKDIR"; \ + pushd $$TARGETDIR >/dev/null; \ + $(ECHO) "$(CYAN)*** Building fuzzer for $@...$(OFF)"; \ + go-fuzz-build -o $$WORKDIR/fuzz.zip; \ + popd >/dev/null; \ fi; \ -go-fuzz -bin=./fuzz-fuzz.zip +if [ "$(FUZZ_BUILD_ONLY)" != "1" ]; then \ + mkdir -p "$$WORKDIR"; \ + cd "$$WORKDIR"; \ + $(ECHO) "$(CYAN)*** Running fuzzer for $@...$(OFF)"; \ + if [ "$(@D)" == "." ]; then \ + go-fuzz -bin=$$WORKDIR/fuzz.zip -workdir=$$WORKDIR; \ + else \ + go-fuzz -bin=$$WORKDIR/fuzz.zip -workdir=$$WORKDIR -func Fuzz$(@F); \ + fi; \ +fi; endef + +# Fuzz consensus transactions. fuzz-consensus: consensus/tendermint/fuzz/ $(canned-fuzz-run) +# Fuzz general storage interface. fuzz-storage: storage/fuzz/ oasis-node/oasis-node @mkdir -p /tmp/oasis-node-fuzz-storage/identity @chmod 0700 /tmp/oasis-node-fuzz-storage/identity @oasis-node/oasis-node identity init --datadir /tmp/oasis-node-fuzz-storage/identity $(canned-fuzz-run) -fuzz-storage-mkvs: storage/mkvs/urkel/fuzz/ +# Fuzz MKVS data structures. +fuzz-mkvs/Tree: storage/mkvs/urkel/fuzz $(canned-fuzz-run) +fuzz-mkvs/Proof: storage/mkvs/urkel/fuzz + $(canned-fuzz-run) + +# Target that only builds all fuzzing infrastructure. +build-fuzz: FUZZ_BUILD_ONLY=1 +build-fuzz: $(fuzz-targets) # Clean. clean: @@ -109,7 +124,7 @@ clean: # List of targets that are not actual files. .PHONY: \ generate $(go-binaries) build \ - $(test-helpers) build-helpers $(test-vectors) gen-test-vectors \ + $(test-helpers) build-helpers $(test-vectors) gen-test-vectors $(fuzz-targets) \ fmt lint test integrationrunner clean all .FORCE: diff --git a/go/common/fuzz/fuzz.go b/go/common/fuzz/fuzz.go index 2d15afd7618..ea9553c6377 100644 --- a/go/common/fuzz/fuzz.go +++ b/go/common/fuzz/fuzz.go @@ -5,105 +5,23 @@ package fuzz import ( "context" - "encoding/binary" "fmt" - "math/rand" "reflect" - gofuzz "github.com/google/gofuzz" + "github.com/thepudds/fzgo/randparam" ) -var ( - _ rand.Source64 = (*Source)(nil) -) - -// Source is a randomness source for the standard random generator. -type Source struct { - Backing []byte - Exhausted int - - pos int - track bool - - traceback []byte -} - -func (s *Source) Int63() int64 { - return int64(s.Uint64()&((1<<63)-1)) -} - -func (s *Source) Seed(_ int64) { - // Nothing to do here. -} - -func (s *Source) Uint64() uint64 { - if s.pos+8 > len(s.Backing) { - s.Exhausted += 8 - r := rand.Uint64() - if s.track { - chunk := make([]byte, 8) - binary.BigEndian.PutUint64(chunk[0:], r) - s.traceback = append(s.traceback, chunk...) - } - return r - } - - s.pos += 8 - return binary.BigEndian.Uint64(s.Backing[s.pos-8 : s.pos]) -} - -// GetTraceback returns the array of bytes returned from the random generator so far. -func (s *Source) GetTraceback() []byte { - return s.traceback -} - -// NewRandSource returns a new random source with the given backing array. -func NewRandSource(backing []byte) *Source { - return &Source{ - Backing: backing, - } -} - -// NewTrackingRandSource returns a new random source that keeps track of the bytes returned. -func NewTrackingRandSource() *Source { - return &Source{ - Backing: []byte{}, - track: true, - } -} - // NewFilledInstance fills the given object with random values from the given blob. -func NewFilledInstance(data []byte, typ interface{}) (interface{}, bool) { +func NewFilledInstance(data []byte, typ interface{}) interface{} { if typ == nil { - return nil, true + return nil } - source := NewRandSource(data) - fuzzer := gofuzz.New() - fuzzer = fuzzer.RandSource(source) - + fuzzer := randparam.NewFuzzer(data) obj := reflect.New(reflect.TypeOf(typ)).Interface() - fuzzer.Fuzz(obj) - return obj, source.Exhausted == 0 -} - -// MakeSampleBlob creates and returns a sample blob of bytes for filling the given object. -func MakeSampleBlob(typ interface{}) []byte { - if typ == nil { - return []byte{} - } - - source := NewTrackingRandSource() - fuzzer := gofuzz.New() - fuzzer = fuzzer.RandSource(source) - - obj := reflect.New(reflect.TypeOf(typ)).Interface() - - fuzzer.Fuzz(obj) - - return source.GetTraceback() + return obj } // InterfaceFuzzer is a helper class for fuzzing methods in structs or interfaces. @@ -140,9 +58,7 @@ func (i *InterfaceFuzzer) DispatchBlob(blob []byte) ([]reflect.Value, bool) { methType := i.typeObject.Method(meth).Type method := i.valObject.Method(meth) - source := NewRandSource(blob[1:]) - fuzzer := gofuzz.New() - fuzzer = fuzzer.RandSource(source).NilChance(0) + fuzzer := randparam.NewFuzzer(blob[1:]) in := []reflect.Value{} @@ -167,34 +83,6 @@ func (i *InterfaceFuzzer) DispatchBlob(blob []byte) ([]reflect.Value, bool) { return method.Call(in), true } -// MakeSampleBlobs returns an array of sample blobs for all methods in the interface. -func (i *InterfaceFuzzer) MakeSampleBlobs() [][]byte { - blobList := [][]byte{} - for seq, meth := range i.methodList { - source := NewTrackingRandSource() - fuzzer := gofuzz.New() - fuzzer = fuzzer.RandSource(source).NilChance(0) - - method := i.typeObject.Method(meth) - blob := []byte{byte(seq)} - for arg := 1; arg < method.Type.NumIn(); arg++ { - inType := method.Type.In(arg) - inTypeName := fmt.Sprintf("%s.%s", inType.PkgPath(), inType.Name()) - if _, ok := i.typeOverrides[inTypeName]; !ok { - newValue := reflect.New(inType) - if newValue.Interface() != nil { - fuzzer.Fuzz(newValue.Interface()) - } - } - } - - blob = append(blob, source.GetTraceback()...) - blobList = append(blobList, blob) - } - - return blobList -} - // Method returns the method object associated with the fuzzer's index-th method for this instance. func (i *InterfaceFuzzer) Method(method int) reflect.Method { return i.typeObject.Method(i.methodList[method]) @@ -202,14 +90,20 @@ func (i *InterfaceFuzzer) Method(method int) reflect.Method { // IgnoreMethodNames makes the interface fuzzer skip the named methods. func (i *InterfaceFuzzer) IgnoreMethodNames(names []string) { - for _, name := range names { - for listIndex, methIndex := range i.methodList { - if i.typeObject.Method(methIndex).Name == name { - i.methodList = append(i.methodList[:listIndex], i.methodList[listIndex+1:]...) - break + var newMethodList []int + +FilterLoop: + for _, index := range i.methodList { + name := i.typeObject.Method(index).Name + for _, ignoreName := range names { + if name == ignoreName { + continue FilterLoop } } + newMethodList = append(newMethodList, index) } + + i.methodList = newMethodList } // NewInterfaceFuzzer creates a new InterfaceFuzzer for the given instance. diff --git a/go/consensus/tendermint/fuzz/.gitignore b/go/consensus/tendermint/fuzz/.gitignore deleted file mode 100644 index c97791ee811..00000000000 --- a/go/consensus/tendermint/fuzz/.gitignore +++ /dev/null @@ -1,5 +0,0 @@ -corpus/ -crashers/ -suppressions/ -*.zip -gencorpus/gencorpus diff --git a/go/consensus/tendermint/fuzz/fuzz.go b/go/consensus/tendermint/fuzz/fuzz.go index 189660bba09..b443abea398 100644 --- a/go/consensus/tendermint/fuzz/fuzz.go +++ b/go/consensus/tendermint/fuzz/fuzz.go @@ -71,7 +71,7 @@ func Fuzz(data []byte) int { methodName := FuzzableMethods[meth] - blob, _ := fuzz.NewFilledInstance(data, methodName.BodyType()) + blob := fuzz.NewFilledInstance(data, methodName.BodyType()) tx := &transaction.Transaction{ Method: methodName, diff --git a/go/consensus/tendermint/fuzz/gencorpus/main.go b/go/consensus/tendermint/fuzz/gencorpus/main.go deleted file mode 100644 index ed3d86b59be..00000000000 --- a/go/consensus/tendermint/fuzz/gencorpus/main.go +++ /dev/null @@ -1,32 +0,0 @@ -// +build gofuzz - -// Gencorpus implements a simple utility to generate corpus files for the fuzzer. -// It has no command-line options and creates the files in the current working directory. -package main - -import ( - "fmt" - "io/ioutil" - - commonFuzz "github.com/oasislabs/oasis-core/go/common/fuzz" - appFuzz "github.com/oasislabs/oasis-core/go/consensus/tendermint/fuzz" -) - -const ( - samplesPerMethod int = 20 -) - -func main() { - for meth := 0; meth < len(appFuzz.FuzzableMethods); meth++ { - for i := 0; i < samplesPerMethod; i++ { - methodName := appFuzz.FuzzableMethods[meth] - - fmt.Println("generating sample", i, "for method", methodName) - - actualSample := []byte{byte(meth)} - actualSample = append(actualSample, commonFuzz.MakeSampleBlob(methodName.BodyType())...) - fileName := fmt.Sprintf("%02d_%s_%02d.bin", meth, string(methodName), i) - _ = ioutil.WriteFile(fileName, actualSample, 0644) - } - } -} diff --git a/go/go.mod b/go/go.mod index fe414cc239d..9c1992b7f1d 100644 --- a/go/go.mod +++ b/go/go.mod @@ -64,6 +64,7 @@ require ( github.com/tendermint/iavl v0.12.2 github.com/tendermint/tendermint v0.32.8 github.com/tendermint/tm-db v0.2.0 + github.com/thepudds/fzgo v0.2.2 github.com/uber-go/atomic v1.4.0 // indirect github.com/uber/jaeger-client-go v2.16.0+incompatible github.com/uber/jaeger-lib v2.0.0+incompatible // indirect diff --git a/go/go.sum b/go/go.sum index c495df216f3..675257dc976 100644 --- a/go/go.sum +++ b/go/go.sum @@ -463,6 +463,8 @@ github.com/tendermint/go-amino v0.15.0 h1:TC4e66P59W7ML9+bxio17CPKnxW3nKIRAYsknt github.com/tendermint/go-amino v0.15.0/go.mod h1:TQU0M1i/ImAo+tYpZi73AU3V/dKeCoMC9Sphe2ZwGME= github.com/tendermint/tm-db v0.2.0 h1:rJxgdqn6fIiVJZy4zLpY1qVlyD0TU6vhkT4kEf71TQQ= github.com/tendermint/tm-db v0.2.0/go.mod h1:0cPKWu2Mou3IlxecH+MEUSYc1Ch537alLe6CpFrKzgw= +github.com/thepudds/fzgo v0.2.2 h1:bGofmgAGfTLpVgETkL9jvhg6azylvCF/kW6JPy5fkzQ= +github.com/thepudds/fzgo v0.2.2/go.mod h1:ZgigL1toyKrar3rWdXz7Fuv7bUpKZ4BAYN49TpEFMCI= github.com/tinylib/msgp v1.1.0 h1:9fQd+ICuRIu/ue4vxJZu6/LzxN0HwMds2nq/0cFvxHU= github.com/tinylib/msgp v1.1.0/go.mod h1:+d+yLhGm8mzTaHzB+wgMYrodPfmZrzkirds8fDWklFE= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= diff --git a/go/storage/fuzz/gencorpus/main.go b/go/storage/fuzz/gencorpus/main.go deleted file mode 100644 index dccd7513370..00000000000 --- a/go/storage/fuzz/gencorpus/main.go +++ /dev/null @@ -1,40 +0,0 @@ -// +build gofuzz - -// Gencorpus implements a simple utility to generate corpus files for the fuzzer. -// It has no command-line options and creates the files in the current working directory. -package main - -import ( - "context" - "fmt" - "io/ioutil" - - "github.com/oasislabs/oasis-core/go/common" - commonFuzz "github.com/oasislabs/oasis-core/go/common/fuzz" - "github.com/oasislabs/oasis-core/go/common/identity" - "github.com/oasislabs/oasis-core/go/storage" -) - -const ( - samplesPerMethod int = 20 -) - -func main() { - storage, err := storage.New(context.Background(), "/tmp/oasis-node-fuzz-storage", common.Namespace{}, &identity.Identity{}, nil, nil) - if err != nil { - panic(err) - } - fuzzer := commonFuzz.NewInterfaceFuzzer(storage) - fuzzer.IgnoreMethodNames([]string{ - "Cleanup", - "Initialized", - }) - - for i := 0; i < samplesPerMethod; i++ { - blobs := fuzzer.MakeSampleBlobs() - for meth := 0; meth < len(blobs); meth++ { - fileName := fmt.Sprintf("%s_%02d.bin", fuzzer.Method(meth).Name, i) - _ = ioutil.WriteFile(fileName, blobs[meth], 0644) - } - } -} diff --git a/go/storage/mkvs/urkel/fuzz/.gitignore b/go/storage/mkvs/urkel/fuzz/.gitignore deleted file mode 100644 index c97791ee811..00000000000 --- a/go/storage/mkvs/urkel/fuzz/.gitignore +++ /dev/null @@ -1,5 +0,0 @@ -corpus/ -crashers/ -suppressions/ -*.zip -gencorpus/gencorpus diff --git a/go/storage/mkvs/urkel/fuzz/gencorpus/main.go b/go/storage/mkvs/urkel/fuzz/gencorpus/main.go deleted file mode 100644 index 140df8083cd..00000000000 --- a/go/storage/mkvs/urkel/fuzz/gencorpus/main.go +++ /dev/null @@ -1,30 +0,0 @@ -// +build gofuzz - -// Gencorpus implements a simple utility to generate corpus files for the fuzzer. -// It has no command-line options and creates the files in the current working directory. -package main - -import ( - "fmt" - "io/ioutil" - - commonFuzz "github.com/oasislabs/oasis-core/go/common/fuzz" - mkvsFuzz "github.com/oasislabs/oasis-core/go/storage/mkvs/urkel/fuzz" -) - -const ( - samplesPerMethod int = 20 -) - -func main() { - tree := mkvsFuzz.NewTreeFuzz() - fuzzer := commonFuzz.NewInterfaceFuzzer(tree) - - for i := 0; i < samplesPerMethod; i++ { - blobs := fuzzer.MakeSampleBlobs() - for meth := 0; meth < len(blobs); meth++ { - fileName := fmt.Sprintf("%s_%02d.bin", fuzzer.Method(meth).Name, i) - _ = ioutil.WriteFile(fileName, blobs[meth], 0644) - } - } -} From 8f15bfae835626b30f988ab0ab36aca8fba0136a Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 21 Feb 2020 13:52:04 +0100 Subject: [PATCH 2/5] go/storage/mkvs: Fuzz remote proof decoder --- .changelog/2637.internal.md | 1 + go/storage/mkvs/urkel/fuzz/proof.go | 42 +++++++++++++++++++ .../mkvs/urkel/fuzz/{fuzz.go => tree.go} | 23 ++++------ 3 files changed, 52 insertions(+), 14 deletions(-) create mode 100644 .changelog/2637.internal.md create mode 100644 go/storage/mkvs/urkel/fuzz/proof.go rename go/storage/mkvs/urkel/fuzz/{fuzz.go => tree.go} (93%) diff --git a/.changelog/2637.internal.md b/.changelog/2637.internal.md new file mode 100644 index 00000000000..2dbde6fc842 --- /dev/null +++ b/.changelog/2637.internal.md @@ -0,0 +1 @@ +go/storage/mkvs: Fuzz storage proof decoder diff --git a/go/storage/mkvs/urkel/fuzz/proof.go b/go/storage/mkvs/urkel/fuzz/proof.go new file mode 100644 index 00000000000..93c043a1877 --- /dev/null +++ b/go/storage/mkvs/urkel/fuzz/proof.go @@ -0,0 +1,42 @@ +// +build gofuzz + +package fuzz + +import ( + "context" + + commonFuzz "github.com/oasislabs/oasis-core/go/common/fuzz" + "github.com/oasislabs/oasis-core/go/storage/mkvs/urkel/syncer" +) + +var proofFuzzer *commonFuzz.InterfaceFuzzer + +// ProofFuzz is a wrapper for fuzzing syncer proof decoding. +type ProofFuzz struct {} + +func (p *ProofFuzz) DecodeProof(ctx context.Context, entries [][]byte) { + var proof syncer.Proof + proof.Entries = entries + + var verifier syncer.ProofVerifier + _, _ = verifier.VerifyProof(ctx, proof.UntrustedRoot, &proof) +} + +func NewProofFuzz() (*ProofFuzz, *commonFuzz.InterfaceFuzzer) { + pf := &ProofFuzz{} + fz := commonFuzz.NewInterfaceFuzzer(pf) + return pf, fz +} + +func init() { + _, proofFuzzer = NewProofFuzz() +} + +func FuzzProof(data []byte) int { + _, result := proofFuzzer.DispatchBlob(data) + if !result { + return -1 + } + + return 0 +} diff --git a/go/storage/mkvs/urkel/fuzz/fuzz.go b/go/storage/mkvs/urkel/fuzz/tree.go similarity index 93% rename from go/storage/mkvs/urkel/fuzz/fuzz.go rename to go/storage/mkvs/urkel/fuzz/tree.go index 3bb71dfd1fa..98c0a0b07c0 100644 --- a/go/storage/mkvs/urkel/fuzz/fuzz.go +++ b/go/storage/mkvs/urkel/fuzz/tree.go @@ -15,11 +15,7 @@ import ( mkvsTests "github.com/oasislabs/oasis-core/go/storage/mkvs/urkel/tests" ) -var ( - tree *TreeFuzz - - fuzzer *commonFuzz.InterfaceFuzzer -) +var treeFuzzer *commonFuzz.InterfaceFuzzer // TreeFuzz is a wrapper around a mkvs.KeyValueTree for fuzzing purposes. type TreeFuzz struct { @@ -177,23 +173,22 @@ func (t *TreeFuzz) fail(format string, a ...interface{}) { panic(fmt.Sprintf(format, a...)) } -func NewTreeFuzz() *TreeFuzz { - return &TreeFuzz{ +func NewTreeFuzz() (*TreeFuzz, *commonFuzz.InterfaceFuzzer) { + tf := &TreeFuzz{ inner: mkvs.New(nil, nil, mkvs.WithoutWriteLog()), reference: make(map[string][]byte), } + fz := commonFuzz.NewInterfaceFuzzer(tf) + return tf, fz } func init() { - // Create the in-memory tree. - tree = NewTreeFuzz() - - // Create and prepare the fuzzer. - fuzzer = commonFuzz.NewInterfaceFuzzer(tree) + // Initialize stateful fuzzing state. + _, treeFuzzer = NewTreeFuzz() } -func Fuzz(data []byte) int { - values, result := fuzzer.DispatchBlob(data) +func FuzzTree(data []byte) int { + values, result := treeFuzzer.DispatchBlob(data) if !result { return -1 } From 0d5d4f5133a746461a4f8dcaa24e018e1d79128d Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 21 Feb 2020 14:05:43 +0100 Subject: [PATCH 3/5] storage/mkvs: Fix node unmarshallers --- .changelog/2703.bugfix.1.md | 1 + go/storage/mkvs/urkel/node/node.go | 12 ++++++++++++ runtime/src/storage/mkvs/urkel/tree/marshal.rs | 13 +++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 .changelog/2703.bugfix.1.md diff --git a/.changelog/2703.bugfix.1.md b/.changelog/2703.bugfix.1.md new file mode 100644 index 00000000000..fb4e96f49e1 --- /dev/null +++ b/.changelog/2703.bugfix.1.md @@ -0,0 +1 @@ +storage/mkvs: Fix node unmarshallers diff --git a/go/storage/mkvs/urkel/node/node.go b/go/storage/mkvs/urkel/node/node.go index 35f711d4e64..fedf642714b 100644 --- a/go/storage/mkvs/urkel/node/node.go +++ b/go/storage/mkvs/urkel/node/node.go @@ -438,10 +438,16 @@ func (n *InternalNode) SizedUnmarshalBinary(data []byte) (int, error) { } labelLen := n.LabelBitLength.ToBytes() pos += DepthSize + if pos+labelLen > len(data) { + return 0, ErrMalformedNode + } n.Label = make(Key, labelLen) copy(n.Label, data[pos:pos+labelLen]) pos += labelLen + if pos >= len(data) { + return 0, ErrMalformedNode + } if data[pos] == PrefixNilNode { n.LeafNode = nil @@ -618,9 +624,15 @@ func (n *LeafNode) SizedUnmarshalBinary(data []byte) (int, error) { return 0, err } pos += keySize + if pos+ValueLengthSize > len(data) { + return 0, ErrMalformedNode + } valueSize := int(binary.LittleEndian.Uint32(data[pos : pos+ValueLengthSize])) pos += ValueLengthSize + if pos+valueSize > len(data) { + return 0, ErrMalformedNode + } value := make([]byte, valueSize) copy(value, data[pos:pos+valueSize]) diff --git a/runtime/src/storage/mkvs/urkel/tree/marshal.rs b/runtime/src/storage/mkvs/urkel/tree/marshal.rs index 7a8cc65cf02..949a3efd4ad 100644 --- a/runtime/src/storage/mkvs/urkel/tree/marshal.rs +++ b/runtime/src/storage/mkvs/urkel/tree/marshal.rs @@ -110,9 +110,15 @@ impl Marshal for InternalNode { pos += self.label_bit_length.unmarshal_binary(&data[pos..])?; self.label = vec![0; self.label_bit_length.to_bytes()]; + if pos + self.label_bit_length.to_bytes() > data.len() { + return Err(TreeError::MalformedNode.into()); + } self.label .clone_from_slice(&data[pos..pos + self.label_bit_length.to_bytes()]); pos += self.label_bit_length.to_bytes(); + if pos >= data.len() { + return Err(TreeError::MalformedNode.into()); + } if data[pos] == NodeKind::None as u8 { self.leaf_node = NodePointer::null_ptr(); @@ -196,11 +202,18 @@ impl Marshal for LeafNode { self.key = Key::new(); let key_len = self.key.unmarshal_binary(&data[pos..])?; pos += key_len; + if pos + VALUE_LENGTH_SIZE > data.len() { + return Err(TreeError::MalformedNode.into()); + } self.value = Value::new(); let mut value_len = 0u32; value_len.unmarshal_binary(&data[pos..(pos + VALUE_LENGTH_SIZE)])?; pos += VALUE_LENGTH_SIZE; + if pos + (value_len as usize) > data.len() { + return Err(TreeError::MalformedNode.into()); + } + self.value .extend_from_slice(&data[pos..(pos + value_len as usize)]); pos += value_len as usize; From dddddc0c02e0f3885ac3dab090cd2536980f3f64 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 21 Feb 2020 14:08:31 +0100 Subject: [PATCH 4/5] storage/mkvs: Fix proof verifier --- .changelog/2703.bugfix.2.md | 1 + go/storage/mkvs/urkel/syncer/proof.go | 5 ++++- runtime/src/storage/mkvs/urkel/sync/proof.rs | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 .changelog/2703.bugfix.2.md diff --git a/.changelog/2703.bugfix.2.md b/.changelog/2703.bugfix.2.md new file mode 100644 index 00000000000..37259d3d0b9 --- /dev/null +++ b/.changelog/2703.bugfix.2.md @@ -0,0 +1 @@ +storage/mkvs: Fix proof verifier diff --git a/go/storage/mkvs/urkel/syncer/proof.go b/go/storage/mkvs/urkel/syncer/proof.go index 3ca9677ca97..dc94b70f58a 100644 --- a/go/storage/mkvs/urkel/syncer/proof.go +++ b/go/storage/mkvs/urkel/syncer/proof.go @@ -187,7 +187,7 @@ func (pv *ProofVerifier) VerifyProof(ctx context.Context, root hash.Hash, proof if !rootNodeHash.Equal(&root) { return nil, fmt.Errorf("verifier: bad root (expected: %s got: %s)", root, - rootNode.Hash, + rootNodeHash, ) } return rootNode, nil @@ -205,6 +205,9 @@ func (pv *ProofVerifier) verifyProof(ctx context.Context, proof *Proof, idx int) if entry == nil { return idx + 1, nil, nil } + if len(entry) == 0 { + return -1, nil, errors.New("verifier: malformed proof") + } switch entry[0] { case proofEntryFull: diff --git a/runtime/src/storage/mkvs/urkel/sync/proof.rs b/runtime/src/storage/mkvs/urkel/sync/proof.rs index 34faa53ab6e..dfec452fbdd 100644 --- a/runtime/src/storage/mkvs/urkel/sync/proof.rs +++ b/runtime/src/storage/mkvs/urkel/sync/proof.rs @@ -103,6 +103,9 @@ impl ProofVerifier { Some(entry) => entry.as_ref(), None => return Ok((idx + 1, NodePointer::null_ptr())), }; + if entry.is_empty() { + return Err(format_err!("verifier: malformed proof")); + } match entry[0] { PROOF_ENTRY_FULL => { From 042174496f26d0dc7f31e935e4ad3562aa0b93b9 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 21 Feb 2020 15:15:47 +0100 Subject: [PATCH 5/5] go/storage/mkvs: Fuzz node unmarshaller --- go/.codecov.yml | 3 +++ go/Makefile | 5 ++++- go/storage/mkvs/urkel/fuzz/node.go | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 go/storage/mkvs/urkel/fuzz/node.go diff --git a/go/.codecov.yml b/go/.codecov.yml index 07b2c0d95db..9e2a2796b68 100644 --- a/go/.codecov.yml +++ b/go/.codecov.yml @@ -5,3 +5,6 @@ ignore: - "oasis-test-runner" # E2E test runner. - "oasis-net-runner" # Test local network runner. - "staking/gen_vectors" # Staking test vector generator. + - "storage/fuzz" # Fuzz tests. + - "storage/mkvs/urkel/fuzz" # Fuzz tests. + - "consensus/tendermint/fuzz" # Fuzz tests. diff --git a/go/Makefile b/go/Makefile index 65624c830c4..49fb18b3093 100644 --- a/go/Makefile +++ b/go/Makefile @@ -73,7 +73,8 @@ integrationrunner: fuzz-targets := fuzz-consensus \ fuzz-storage \ fuzz-mkvs/Tree \ - fuzz-mkvs/Proof + fuzz-mkvs/Proof \ + fuzz-mkvs/Node define canned-fuzz-run @TARGETDIR=$(shell pwd)/$<; \ @@ -111,6 +112,8 @@ fuzz-mkvs/Tree: storage/mkvs/urkel/fuzz $(canned-fuzz-run) fuzz-mkvs/Proof: storage/mkvs/urkel/fuzz $(canned-fuzz-run) +fuzz-mkvs/Node: storage/mkvs/urkel/fuzz + $(canned-fuzz-run) # Target that only builds all fuzzing infrastructure. build-fuzz: FUZZ_BUILD_ONLY=1 diff --git a/go/storage/mkvs/urkel/fuzz/node.go b/go/storage/mkvs/urkel/fuzz/node.go new file mode 100644 index 00000000000..f277e72aa2b --- /dev/null +++ b/go/storage/mkvs/urkel/fuzz/node.go @@ -0,0 +1,18 @@ +// +build gofuzz + +package fuzz + +import "github.com/oasislabs/oasis-core/go/storage/mkvs/urkel/node" + +func FuzzNode(data []byte) int { + n, err := node.UnmarshalBinary(data) + if err != nil { + return 0 + } + + _, err = n.CompactMarshalBinary() + if err != nil { + panic(err) + } + return 1 +}