Patch forging empty merkle tree attack vector

crypto/merkle/proof.go

@@ -60,15 +60,18 @@ 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)
 	}
 	return nil
 }
 
 // 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
 	}
 }
 

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 {

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
+}