fix(go): bullet-proof against nil dereferences + more fuzzers

go/fuzz_test.go

@@ -1,6 +1,7 @@
 package ics23
 
 import (
+	"bytes"
 	"encoding/json"
 	"os"
 	"path/filepath"
@@ -40,6 +41,38 @@ func FuzzExistenceProofCalculate(f *testing.F) {
 	})
 }
 
+func FuzzExistenceProofCheckAgainstSpec(f *testing.F) {
+	if testing.Short() {
+		f.Skip("in -short mode")
+	}
+
+	seedDataMap := CheckAgainstSpecTestData(f)
+	for _, seed := range seedDataMap {
+		if seed.IsErr {
+			// Erroneous data, skip it.
+			continue
+		}
+		if blob, err := json.Marshal(seed); err == nil {
+			f.Add(blob)
+		}
+	}
+
+	// 2. Now run the fuzzer.
+	f.Fuzz(func(t *testing.T, fJSON []byte) {
+		pvs := new(CheckAgainstSpecTestStruct)
+		if err := json.Unmarshal(fJSON, pvs); err != nil {
+			return
+		}
+		if pvs.Proof == nil {
+			// No need checking from a nil ExistenceProof.
+			return
+		}
+
+		ep, spec := pvs.Proof, pvs.Spec
+		_ = ep.CheckAgainstSpec(spec)
+	})
+}
+
 var batchVectorDataSeeds []*BatchVectorData
 
 func init() {
@@ -89,6 +122,57 @@ func FuzzVerifyNonMembership(f *testing.F) {
 	})
 }
 
+// verifyJSON is necessary so we already have sources of Proof, Ref and Spec
+// that'll be mutated by the fuzzer to get much closer to values for greater coverage.
+type verifyJSON struct {
+	Proof *CommitmentProof
+	Ref   *RefData
+	Spec  *ProofSpec
+}
+
+func FuzzVerifyMembership(f *testing.F) {
+	seeds := VectorsTestData()
+
+	// VerifyMembership requires:
+	//  Spec, ExistenceProof, CommitmentRootref.
+	for _, seed := range seeds {
+		proof, ref := LoadFile(f, seed.Dir, seed.Filename)
+		root, err := proof.Calculate()
+		if err != nil {
+			continue
+		}
+		if !bytes.Equal(ref.RootHash, root) {
+			continue
+		}
+
+		if ref.Value == nil {
+			continue
+		}
+
+		// Now serialize this value as a seed.
+		// The use of already calculated values is necessary
+		// for the fuzzers to have a basis of much better coverage
+		// generating values.
+		blob, err := json.Marshal(&verifyJSON{Proof: proof, Ref: ref, Spec: seed.Spec})
+		if err == nil {
+			f.Add(blob)
+		}
+	}
+
+	// 2. Now let's run the fuzzer.
+	f.Fuzz(func(t *testing.T, input []byte) {
+		var con verifyJSON
+		if err := json.Unmarshal(input, &con); err != nil {
+			return
+		}
+		spec, ref, proof := con.Spec, con.Ref, con.Proof
+		if ref == nil {
+			return
+		}
+		_ = VerifyMembership(spec, ref.RootHash, proof, ref.Key, ref.Value)
+	})
+}
+
 func FuzzCombineProofs(f *testing.F) {
 	// 1. Load in the CommitmentProofs
 	baseDirs := []string{"iavl", "tendermint", "smt"}

go/ics23.go

@@ -132,6 +132,10 @@ func CombineProofs(proofs []*CommitmentProof) (*CommitmentProof, error) {
 }
 
 func getExistProofForKey(proof *CommitmentProof, key []byte) *ExistenceProof {
+	if proof == nil {
+		return nil
+	}
+
 	switch p := proof.Proof.(type) {
 	case *CommitmentProof_Exist:
 		ep := p.Exist

go/ops.go

@@ -94,7 +94,13 @@ func (op *InnerOp) Apply(child []byte) ([]byte, error) {
 
 // CheckAgainstSpec will verify the LeafOp is in the format defined in spec
 func (op *LeafOp) CheckAgainstSpec(spec *ProofSpec) error {
+	if spec == nil {
+		return errors.New("op and spec must be non-nil")
+	}
 	lspec := spec.LeafSpec
+	if lspec == nil {
+		return errors.New("spec.LeafSpec must be non-nil")
+	}
 
 	if validateSpec(spec) {
 		err := validateIavlOps(op, 0)
@@ -123,6 +129,16 @@ func (op *LeafOp) CheckAgainstSpec(spec *ProofSpec) error {
 
 // CheckAgainstSpec will verify the InnerOp is in the format defined in spec
 func (op *InnerOp) CheckAgainstSpec(spec *ProofSpec, b int) error {
+	if spec == nil {
+		return errors.New("op and spec must be both non-nil")
+	}
+	if spec.InnerSpec == nil {
+		return errors.New("spec.InnerSpec must be non-nil")
+	}
+	if spec.LeafSpec == nil {
+		return errors.New("spec.LeafSpec must be non-nil")
+	}
+
 	if op.Hash != spec.InnerSpec.Hash {
 		return fmt.Errorf("unexpected HashOp: %d", op.Hash)
 	}
@@ -146,6 +162,10 @@ func (op *InnerOp) CheckAgainstSpec(spec *ProofSpec, b int) error {
 		return fmt.Errorf("innerOp prefix too long (%d)", len(op.Prefix))
 	}
 
+	if spec.InnerSpec.ChildSize <= 0 {
+		return errors.New("spec.InnerSpec.ChildSize must be >= 1")
+	}
+
 	// ensures soundness, with suffix having to be of correct length
 	if len(op.Suffix)%int(spec.InnerSpec.ChildSize) != 0 {
 		return fmt.Errorf("InnerOp suffix malformed")

go/proof.go

@@ -180,11 +180,11 @@ func (p *NonExistenceProof) Calculate() (CommitmentRoot, error) {
 
 // CheckAgainstSpec will verify the leaf and all path steps are in the format defined in spec
 func (p *ExistenceProof) CheckAgainstSpec(spec *ProofSpec) error {
-	if p.GetLeaf() == nil {
+	leaf := p.GetLeaf()
+	if leaf == nil {
 		return errors.New("existence Proof needs defined LeafOp")
 	}
-	err := p.Leaf.CheckAgainstSpec(spec)
-	if err != nil {
+	if err := leaf.CheckAgainstSpec(spec); err != nil {
 		return fmt.Errorf("leaf, %w", err)
 	}
 	if spec.MinDepth > 0 && len(p.Path) < int(spec.MinDepth) {
@@ -452,13 +452,41 @@ func orderFromPadding(spec *InnerSpec, inner *InnerOp) (int32, error) {
 
 // over-declares equality, which we cosnider fine for now.
 func (p *ProofSpec) SpecEquals(spec *ProofSpec) bool {
-	return p.LeafSpec.Hash == spec.LeafSpec.Hash &&
-		p.LeafSpec.PrehashKey == spec.LeafSpec.PrehashKey &&
-		p.LeafSpec.PrehashValue == spec.LeafSpec.PrehashValue &&
-		p.LeafSpec.Length == spec.LeafSpec.Length &&
-		p.InnerSpec.Hash == spec.InnerSpec.Hash &&
-		p.InnerSpec.MinPrefixLength == spec.InnerSpec.MinPrefixLength &&
-		p.InnerSpec.MaxPrefixLength == spec.InnerSpec.MaxPrefixLength &&
-		p.InnerSpec.ChildSize == spec.InnerSpec.ChildSize &&
-		len(p.InnerSpec.ChildOrder) == len(spec.InnerSpec.ChildOrder)
+	// 1. Compare LeafSpecs values.
+	switch {
+	case (p.LeafSpec == nil) != (spec.LeafSpec == nil): // One of them is nil.
+		return false
+
+	case p.LeafSpec != nil && spec.LeafSpec != nil:
+		ok := p.LeafSpec.Hash == spec.LeafSpec.Hash &&
+			p.LeafSpec.PrehashKey == spec.LeafSpec.PrehashKey &&
+			p.LeafSpec.PrehashValue == spec.LeafSpec.PrehashValue &&
+			p.LeafSpec.Length == spec.LeafSpec.Length
+		if !ok {
+			return false
+		}
+
+	default: // Both are nil, hence LeafSpec values are equal.
+	}
+
+	// 2. Compare InnerSpec values.
+	switch {
+	case (p.InnerSpec == nil) != (spec.InnerSpec == nil): // One of them is not nil.
+		return false
+
+	case p.InnerSpec != nil && spec.InnerSpec != nil: // Both are non-nil
+		ok := p.InnerSpec.Hash == spec.InnerSpec.Hash &&
+			p.InnerSpec.MinPrefixLength == spec.InnerSpec.MinPrefixLength &&
+			p.InnerSpec.MaxPrefixLength == spec.InnerSpec.MaxPrefixLength &&
+			p.InnerSpec.ChildSize == spec.InnerSpec.ChildSize &&
+			len(p.InnerSpec.ChildOrder) == len(spec.InnerSpec.ChildOrder)
+		if !ok {
+			return false
+		}
+
+	default: // Both are nil, hence InnerSpec values are equal.
+	}
+
+	// By this point all the above conditions pass so they are equal.
+	return true
 }

go/proof_data_test.go

@@ -13,18 +13,18 @@ type ExistenceProofTestStruct struct {
 	Expected []byte
 }
 
-func ExistenceProofTestData(t *testing.T) map[string]ExistenceProofTestStruct {
-	t.Helper()
+func ExistenceProofTestData(tb testing.TB) map[string]ExistenceProofTestStruct {
+	tb.Helper()
 	fname := filepath.Join("..", "testdata", "TestExistenceProofData.json")
 	ffile, err := os.Open(fname)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	var cases map[string]ExistenceProofTestStruct
 	jsonDecoder := json.NewDecoder(ffile)
 	err = jsonDecoder.Decode(&cases)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	return cases
 }
@@ -35,18 +35,18 @@ type CheckLeafTestStruct struct {
 	IsErr bool
 }
 
-func CheckLeafTestData(t *testing.T) map[string]CheckLeafTestStruct {
-	t.Helper()
+func CheckLeafTestData(tb testing.TB) map[string]CheckLeafTestStruct {
+	tb.Helper()
 	fname := filepath.Join("..", "testdata", "TestCheckLeafData.json")
 	ffile, err := os.Open(fname)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	var cases map[string]CheckLeafTestStruct
 	jsonDecoder := json.NewDecoder(ffile)
 	err = jsonDecoder.Decode(&cases)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	return cases
 }
@@ -57,18 +57,18 @@ type CheckAgainstSpecTestStruct struct {
 	IsErr bool
 }
 
-func CheckAgainstSpecTestData(t *testing.T) map[string]CheckAgainstSpecTestStruct {
-	t.Helper()
+func CheckAgainstSpecTestData(tb testing.TB) map[string]CheckAgainstSpecTestStruct {
+	tb.Helper()
 	fname := filepath.Join("..", "testdata", "TestCheckAgainstSpecData.json")
 	ffile, err := os.Open(fname)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	var cases map[string]CheckAgainstSpecTestStruct
 	jsonDecoder := json.NewDecoder(ffile)
 	err = jsonDecoder.Decode(&cases)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	return cases
 }

go/testdata/fuzz/FuzzExistenceProofCheckAgainstSpec/09bebc2fc8d0a79b

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("{\"Proof\":{\"leAf\":{\"prehAsh_vAlue\":1,\"length\":1,\"prefiX\":\"AA00\"},\"pAth\":[{\"hAsh\":1}]},\"SpeC\":{\"leAf_speC\":{\"prehAsh_vAlue\":1,\"length\":1,\"prefiX\":\"AA00\"},\"inner_speC\":{\"hAsh\":1}}}")

go/testdata/fuzz/FuzzExistenceProofCheckAgainstSpec/19e35d361fe85847

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("{\"Proof\":{\"leAf\":{},\"pAth\":[{}]},\"SpeC\":{\"leAf_speC\":{}}}")

go/testdata/fuzz/FuzzExistenceProofCheckAgainstSpec/1f84363823f5c624

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("{\"Proof\":{\"leAf\":{}},\"SpeC\":{}}")

go/testdata/fuzz/FuzzExistenceProofCheckAgainstSpec/a9ba9cba7c7724a0

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("{\"Proof\":{\"leAf\":{}}}")

go/testdata/fuzz/FuzzVerifyMembership/8093511184ad3e25

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("{}")

go/testdata/fuzz/FuzzVerifyMembership/99dd1125ca292163

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("{\"Ref\":{}}")

go/vectors_data_test.go

@@ -165,42 +165,42 @@ func DecompressBatchVectorsTestData(t *testing.T) map[string]*CommitmentProof {
 	}
 }
 
-func LoadFile(t *testing.T, dir string, filename string) (*CommitmentProof, *RefData) {
-	t.Helper()
+func LoadFile(tb testing.TB, dir string, filename string) (*CommitmentProof, *RefData) {
+	tb.Helper()
 	// load the file into a json struct
 	name := filepath.Join(dir, filename)
 	bz, err := os.ReadFile(name)
 	if err != nil {
-		t.Fatalf("Read file: %+v", err)
+		tb.Fatalf("Read file: %+v", err)
 	}
 	var data TestVector
 	err = json.Unmarshal(bz, &data)
 	if err != nil {
-		t.Fatalf("Unmarshal json: %+v", err)
+		tb.Fatalf("Unmarshal json: %+v", err)
 	}
 	// parse the protobuf object
 	var proof CommitmentProof
-	err = proof.Unmarshal(mustHex(t, data.Proof))
+	err = proof.Unmarshal(mustHex(tb, data.Proof))
 	if err != nil {
-		t.Fatalf("Unmarshal protobuf: %+v", err)
+		tb.Fatalf("Unmarshal protobuf: %+v", err)
 	}
 	var ref RefData
-	ref.RootHash = CommitmentRoot(mustHex(t, data.RootHash))
-	ref.Key = mustHex(t, data.Key)
+	ref.RootHash = CommitmentRoot(mustHex(tb, data.RootHash))
+	ref.Key = mustHex(tb, data.Key)
 	if data.Value != "" {
-		ref.Value = mustHex(t, data.Value)
+		ref.Value = mustHex(tb, data.Value)
 	}
 	return &proof, &ref
 }
 
-func mustHex(t *testing.T, data string) []byte {
-	t.Helper()
+func mustHex(tb testing.TB, data string) []byte {
+	tb.Helper()
 	if data == "" {
 		return nil
 	}
 	res, err := hex.DecodeString(data)
 	if err != nil {
-		t.Fatalf("decoding hex: %v", err)
+		tb.Fatalf("decoding hex: %v", err)
 	}
 	return res
 }