fix: prevent out of bounds on lookup inners access

go/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - deps: bump golang to v1.22 ([#363](https://github.com/cosmos/ics23/pull/363)).
 - fix: guarantee that `spec.InnerSpec.MaxPrefixLength` < `spec.InnerSpec.MinPrefixLength` + `spec.InnerSpec.ChildSize` ([#369](https://github.com/cosmos/ics23/pull/369))
+- imp: change panics on out of bound inner op lookups in compressed batch proofs to error returns. The `Decompress` function now returns an error, along with `decompress`, `decompressEntry`, and `decompressExist`. 
 
 # v0.11.0
 

go/compress.go

@@ -1,5 +1,7 @@
 package ics23
 
+import "fmt"
+
 // IsCompressed returns true if the proof was compressed
 func IsCompressed(proof *CommitmentProof) bool {
 	return proof.GetCompressed() != nil
@@ -23,16 +25,20 @@ func Compress(proof *CommitmentProof) *CommitmentProof {
 // Decompress will return a BatchProof if the input is CompressedBatchProof
 // Otherwise it will return the input.
 // This is safe to call multiple times (idempotent)
-func Decompress(proof *CommitmentProof) *CommitmentProof {
+func Decompress(proof *CommitmentProof) (*CommitmentProof, error) {
 	comp := proof.GetCompressed()
 	if comp != nil {
+		batch, err := decompress(comp)
+		if err != nil {
+			return nil, err
+		}
 		return &CommitmentProof{
 			Proof: &CommitmentProof_Batch{
-				Batch: decompress(comp),
+				Batch: batch,
 			},
-		}
+		}, nil
 	}
-	return proof
+	return proof, nil
 }
 
 func compress(batch *BatchProof) *CompressedBatchProof {
@@ -106,45 +112,62 @@ func compressStep(step *InnerOp, lookup *[]*InnerOp, registry map[string]int32)
 	return num
 }
 
-func decompress(comp *CompressedBatchProof) *BatchProof {
+func decompress(comp *CompressedBatchProof) (*BatchProof, error) {
 	lookup := comp.LookupInners
 
 	var entries []*BatchEntry
 
 	for _, centry := range comp.Entries {
-		entry := decompressEntry(centry, lookup)
+		entry, err := decompressEntry(centry, lookup)
+		if err != nil {
+			return nil, err
+		}
 		entries = append(entries, entry)
 	}
 
 	return &BatchProof{
 		Entries: entries,
-	}
+	}, nil
 }
 
-func decompressEntry(entry *CompressedBatchEntry, lookup []*InnerOp) *BatchEntry {
+func decompressEntry(entry *CompressedBatchEntry, lookup []*InnerOp) (*BatchEntry, error) {
 	if exist := entry.GetExist(); exist != nil {
+		decompressedExist, err := decompressExist(exist, lookup)
+		if err != nil {
+			return nil, err
+		}
 		return &BatchEntry{
 			Proof: &BatchEntry_Exist{
-				Exist: decompressExist(exist, lookup),
+				Exist: decompressedExist,
 			},
-		}
+		}, nil
 	}
 
 	non := entry.GetNonexist()
+	decompressedLeft, err := decompressExist(non.Left, lookup)
+	if err != nil {
+		return nil, err
+	}
+
+	decompressedRight, err := decompressExist(non.Right, lookup)
+	if err != nil {
+		return nil, err
+	}
+
 	return &BatchEntry{
 		Proof: &BatchEntry_Nonexist{
 			Nonexist: &NonExistenceProof{
 				Key:   non.Key,
-				Left:  decompressExist(non.Left, lookup),
-				Right: decompressExist(non.Right, lookup),
+				Left:  decompressedLeft,
+				Right: decompressedRight,
 			},
 		},
-	}
+	}, nil
 }
 
-func decompressExist(exist *CompressedExistenceProof, lookup []*InnerOp) *ExistenceProof {
+func decompressExist(exist *CompressedExistenceProof, lookup []*InnerOp) (*ExistenceProof, error) {
 	if exist == nil {
-		return nil
+		return nil, nil
 	}
 	res := &ExistenceProof{
 		Key:   exist.Key,
@@ -153,7 +176,10 @@ func decompressExist(exist *CompressedExistenceProof, lookup []*InnerOp) *Existe
 		Path:  make([]*InnerOp, len(exist.Path)),
 	}
 	for i, step := range exist.Path {
+		if int(step) >= len(lookup) {
+			return nil, fmt.Errorf("compressed existence proof at index %d has lookup index out of bounds", i)
+		}
 		res.Path[i] = lookup[step]
 	}
-	return res
+	return res, nil
 }

go/compress_test.go

@@ -0,0 +1,119 @@
+package ics23
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+)
+
+func TestDecompressExist(t *testing.T) {
+	leafOp := &LeafOp{
+		Hash:         HashOp_SHA256,
+		PrehashKey:   HashOp_NO_HASH,
+		PrehashValue: HashOp_NO_HASH,
+		Length:       LengthOp_NO_PREFIX,
+		Prefix:       []byte{},
+	}
+	innerOps := []*InnerOp{
+		{
+			Hash:   HashOp_SHA256,
+			Prefix: generateInnerOpPrefix(),
+		},
+		{
+			Hash:   HashOp_SHA256,
+			Prefix: generateInnerOpPrefix(),
+			Suffix: []byte{1},
+		},
+		{
+			Hash:   HashOp_SHA256,
+			Prefix: generateInnerOpPrefix(),
+			Suffix: []byte{2},
+		},
+	}
+
+	var (
+		compressedExistProof *CompressedExistenceProof
+		lookup               []*InnerOp
+	)
+
+	cases := []struct {
+		name     string
+		malleate func()
+		expProof *ExistenceProof
+		expError error
+	}{
+		{
+			"success: single lookup",
+			func() {
+				compressedExistProof.Path = []int32{0}
+			},
+			&ExistenceProof{
+				Key:   []byte{0},
+				Value: []byte{0},
+				Leaf:  leafOp,
+				Path:  []*InnerOp{innerOps[0]},
+			},
+			nil,
+		},
+		{
+			"success: multiple lookups",
+			func() {
+				compressedExistProof.Path = []int32{0, 1, 0, 2}
+			},
+			&ExistenceProof{
+				Key:   []byte{0},
+				Value: []byte{0},
+				Leaf:  leafOp,
+				Path:  []*InnerOp{innerOps[0], innerOps[1], innerOps[0], innerOps[2]},
+			},
+			nil,
+		},
+		{
+			"success: empty exist proof",
+			func() {
+				compressedExistProof = nil
+			},
+			nil,
+			nil,
+		},
+		{
+			"failure: path index out of bounds",
+			func() {
+				compressedExistProof.Path = []int32{0}
+				lookup = nil
+			},
+			nil,
+			fmt.Errorf("compressed existence proof at index %d has lookup index out of bounds", 0),
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			// reset default values for compressedExistProof and lookup
+			compressedExistProof = &CompressedExistenceProof{
+				Key:   []byte{0},
+				Value: []byte{0},
+				Leaf:  leafOp,
+			}
+
+			lookup = innerOps
+
+			tc.malleate()
+
+			proof, err := decompressExist(compressedExistProof, lookup)
+			if !reflect.DeepEqual(tc.expProof, proof) {
+				t.Fatalf("expexted proof: %v, got: %v", tc.expProof, proof)
+			}
+
+			if tc.expError == nil && err != nil {
+				t.Fatal(err)
+			}
+
+			if tc.expError != nil && err.Error() != tc.expError.Error() {
+				t.Fatalf("expected: %v, got: %v", tc.expError, err)
+			}
+		})
+	}
+}

go/ics23.go

@@ -35,12 +35,15 @@ type CommitmentRoot []byte
 // calculating the root for the ExistenceProof matches the provided CommitmentRoot
 func VerifyMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentProof, key []byte, value []byte) bool {
 	// decompress it before running code (no-op if not compressed)
-	proof = Decompress(proof)
+	proof, err := Decompress(proof)
+	if err != nil {
+		return false
+	}
 	ep := getExistProofForKey(proof, key)
 	if ep == nil {
 		return false
 	}
-	err := ep.Verify(spec, root, key, value)
+	err = ep.Verify(spec, root, key, value)
 	return err == nil
 }
 
@@ -51,20 +54,26 @@ func VerifyMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentPro
 // provided key is between the keys of the two proofs
 func VerifyNonMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentProof, key []byte) bool {
 	// decompress it before running code (no-op if not compressed)
-	proof = Decompress(proof)
+	proof, err := Decompress(proof)
+	if err != nil {
+		return false
+	}
 	np := getNonExistProofForKey(spec, proof, key)
 	if np == nil {
 		return false
 	}
-	err := np.Verify(spec, root, key)
+	err = np.Verify(spec, root, key)
 	return err == nil
 }
 
 // BatchVerifyMembership will ensure all items are also proven by the CommitmentProof (which should be a BatchProof,
 // unless there is one item, when a ExistenceProof may work)
 func BatchVerifyMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentProof, items map[string][]byte) bool {
 	// decompress it before running code (no-op if not compressed) - once for batch
-	proof = Decompress(proof)
+	proof, err := Decompress(proof)
+	if err != nil {
+		return false
+	}
 	for k, v := range items {
 		valid := VerifyMembership(spec, root, proof, []byte(k), v)
 		if !valid {
@@ -78,7 +87,10 @@ func BatchVerifyMembership(spec *ProofSpec, root CommitmentRoot, proof *Commitme
 // (which should be a BatchProof, unless there is one item, when a NonExistenceProof may work)
 func BatchVerifyNonMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentProof, keys [][]byte) bool {
 	// decompress it before running code (no-op if not compressed) - once for batch
-	proof = Decompress(proof)
+	proof, err := Decompress(proof)
+	if err != nil {
+		return false
+	}
 	for _, k := range keys {
 		valid := VerifyNonMembership(spec, root, proof, k)
 		if !valid {
@@ -113,7 +125,10 @@ func CombineProofs(proofs []*CommitmentProof) (*CommitmentProof, error) {
 		} else if batch := proof.GetBatch(); batch != nil {
 			entries = append(entries, batch.Entries...)
 		} else if comp := proof.GetCompressed(); comp != nil {
-			decomp := Decompress(proof)
+			decomp, err := Decompress(proof)
+			if err != nil {
+				return nil, err
+			}
 			entries = append(entries, decomp.GetBatch().Entries...)
 		} else {
 			return nil, fmt.Errorf("proof neither exist or nonexist: %#v", proof.GetProof())

go/proof.go

@@ -97,7 +97,10 @@ func (p *CommitmentProof) Calculate() (CommitmentRoot, error) {
 			return n.Calculate()
 		}
 	case *CommitmentProof_Compressed:
-		proof := Decompress(p)
+		proof, err := Decompress(p)
+		if err != nil {
+			return nil, err
+		}
 		return proof.Calculate()
 	default:
 		return nil, errors.New("unrecognized proof type")

go/vectors_test.go

@@ -83,7 +83,10 @@ func TestDecompressBatchVectors(t *testing.T) {
 				t.Fatalf("Marshal batch %v", err)
 			}
 
-			decomp := Decompress(tc)
+			decomp, err := Decompress(tc)
+			if err != nil {
+				t.Fatalf("Failed to decompress test case")
+			}
 			if decomp == tc {
 				t.Fatalf("Decompression is a no-op")
 			}