diff --git a/go/CHANGELOG.md b/go/CHANGELOG.md index d49eff71..7f4fe666 100644 --- a/go/CHANGELOG.md +++ b/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 diff --git a/go/compress.go b/go/compress.go index 173e7c87..0755a695 100644 --- a/go/compress.go +++ b/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,64 @@ 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) { + // when proving non-existence, if we are proving the leftmost key or rightmost key, + // the left existence proof or right existence proof will be nil, so we safely return. if exist == nil { - return nil + return nil, nil } res := &ExistenceProof{ Key: exist.Key, @@ -153,7 +178,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 } diff --git a/go/compress_test.go b/go/compress_test.go new file mode 100644 index 00000000..6e154c8c --- /dev/null +++ b/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) + } + }) + } +} diff --git a/go/ics23.go b/go/ics23.go index 709094d6..16108464 100644 --- a/go/ics23.go +++ b/go/ics23.go @@ -36,12 +36,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 } @@ -52,12 +55,15 @@ 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 } @@ -65,7 +71,10 @@ func VerifyNonMembership(spec *ProofSpec, root CommitmentRoot, proof *Commitment // 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 { @@ -79,7 +88,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 { @@ -114,7 +126,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()) diff --git a/go/proof.go b/go/proof.go index 91fa232e..7f8e5120 100644 --- a/go/proof.go +++ b/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") diff --git a/go/vectors_test.go b/go/vectors_test.go index b6e0e909..5e1d6767 100644 --- a/go/vectors_test.go +++ b/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") }