From 77d657e65991e5b0c83d07378a90c7ed268384db Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 20 Mar 2023 11:19:05 +0800 Subject: [PATCH] Patch forging empty merkle tree attack vector --- crypto/merkle/proof.go | 33 +++++++++++++++++--------------- crypto/merkle/proof_value.go | 8 +++++--- crypto/merkle/tree_test.go | 37 ++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/crypto/merkle/proof.go b/crypto/merkle/proof.go index 8b98d1b21..f47e5927b 100644 --- a/crypto/merkle/proof.go +++ b/crypto/merkle/proof.go @@ -60,7 +60,10 @@ func (sp *Proof) Verify(rootHash []byte, leaf []byte) error { if !bytes.Equal(sp.LeafHash, leafHash) { return fmt.Errorf("invalid leaf hash: wanted %X got %X", leafHash, sp.LeafHash) } - computedHash := sp.ComputeRootHash() + computedHash, err := sp.ComputeRootHash() + if err != nil { + return err + } if !bytes.Equal(computedHash, rootHash) { return fmt.Errorf("invalid root hash: wanted %X got %X", rootHash, computedHash) } @@ -68,7 +71,7 @@ func (sp *Proof) Verify(rootHash []byte, leaf []byte) error { } // Compute the root hash given a leaf hash. Does not verify the result. -func (sp *Proof) ComputeRootHash() []byte { +func (sp *Proof) ComputeRootHash() ([]byte, error) { return computeHashFromAunts( sp.Index, sp.Total, @@ -148,35 +151,35 @@ func ProofFromProto(pb *tmcrypto.Proof) (*Proof, error) { // Use the leafHash and innerHashes to get the root merkle hash. // If the length of the innerHashes slice isn't exactly correct, the result is nil. // Recursive impl. -func computeHashFromAunts(index, total int64, leafHash []byte, innerHashes [][]byte) []byte { +func computeHashFromAunts(index, total int64, leafHash []byte, innerHashes [][]byte) ([]byte, error) { if index >= total || index < 0 || total <= 0 { - return nil + return nil, fmt.Errorf("Calling computeHashFromAunts() with invalid index (%d) and total (%d)", index, total) } switch total { case 0: panic("Cannot call computeHashFromAunts() with 0 total") case 1: if len(innerHashes) != 0 { - return nil + return nil, errors.New("Calling computeHashFromAunts() with total 1 but non-empty inner hashes") } - return leafHash + return leafHash, nil default: if len(innerHashes) == 0 { - return nil + return nil, errors.New("Calling computeHashFromAunts() with total > 1 but empty inner hashes") } numLeft := getSplitPoint(total) if index < numLeft { - leftHash := computeHashFromAunts(index, numLeft, leafHash, innerHashes[:len(innerHashes)-1]) - if leftHash == nil { - return nil + leftHash, err := computeHashFromAunts(index, numLeft, leafHash, innerHashes[:len(innerHashes)-1]) + if err != nil { + return nil, err } - return innerHash(leftHash, innerHashes[len(innerHashes)-1]) + return innerHash(leftHash, innerHashes[len(innerHashes)-1]), nil } - rightHash := computeHashFromAunts(index-numLeft, total-numLeft, leafHash, innerHashes[:len(innerHashes)-1]) - if rightHash == nil { - return nil + rightHash, err := computeHashFromAunts(index-numLeft, total-numLeft, leafHash, innerHashes[:len(innerHashes)-1]) + if err != nil { + return nil, err } - return innerHash(innerHashes[len(innerHashes)-1], rightHash) + return innerHash(innerHashes[len(innerHashes)-1], rightHash), nil } } diff --git a/crypto/merkle/proof_value.go b/crypto/merkle/proof_value.go index 0f4f2eb3d..33ee789bc 100644 --- a/crypto/merkle/proof_value.go +++ b/crypto/merkle/proof_value.go @@ -92,9 +92,11 @@ func (op ValueOp) Run(args [][]byte) ([][]byte, error) { return nil, fmt.Errorf("leaf hash mismatch: want %X got %X", op.Proof.LeafHash, kvhash) } - return [][]byte{ - op.Proof.ComputeRootHash(), - }, nil + rootHash, err := op.Proof.ComputeRootHash() + if err != nil { + return nil, err + } + return [][]byte{rootHash}, nil } func (op ValueOp) GetKey() []byte { diff --git a/crypto/merkle/tree_test.go b/crypto/merkle/tree_test.go index 72b260178..56ed4f9bd 100644 --- a/crypto/merkle/tree_test.go +++ b/crypto/merkle/tree_test.go @@ -1,13 +1,17 @@ package merkle import ( + "bytes" + "encoding/binary" "encoding/hex" + "io" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/tmhash" ctest "github.com/tendermint/tendermint/internal/libs/test" tmrand "github.com/tendermint/tendermint/libs/rand" ) @@ -114,6 +118,23 @@ func TestHashAlternatives(t *testing.T) { require.Equal(t, rootHash1, rootHash2, "Unmatched root hashes: %X vs %X", rootHash1, rootHash2) } +// See https://blog.verichains.io/p/vsa-2022-100-tendermint-forging-membership-proof?utm_source=substack&utm_campaign=post_embed&utm_medium=web +// for context +func TestForgeEmptyMerkleTreeAttack(t *testing.T) { + key := []byte{0x13} + value := []byte{0x37} + vhash := tmhash.Sum(value) + bz := new(bytes.Buffer) + _ = EncodeByteSlice(bz, key) + _ = EncodeByteSlice(bz, vhash) + kvhash := tmhash.Sum(append([]byte{0}, bz.Bytes()...)) + op := NewValueOp(key, &Proof{LeafHash: kvhash}) + var root []byte + err := ProofOperators{op}.Verify(root, "/"+string(key), [][]byte{value}) + // Must return error or else the attack would be possible + require.NotNil(t, err) +} + func BenchmarkHashAlternatives(b *testing.B) { total := 100 @@ -158,3 +179,19 @@ func Test_getSplitPoint(t *testing.T) { require.EqualValues(t, tt.want, got, "getSplitPoint(%d) = %v, want %v", tt.length, got, tt.want) } } + +func EncodeUvarint(w io.Writer, u uint64) (err error) { + var buf [10]byte + n := binary.PutUvarint(buf[:], u) + _, err = w.Write(buf[0:n]) + return +} + +func EncodeByteSlice(w io.Writer, bz []byte) (err error) { + err = EncodeUvarint(w, uint64(len(bz))) + if err != nil { + return + } + _, err = w.Write(bz) + return +}