sec: add proof ops check and key checker

core/vm/contracts_lightclient.go

@@ -3,6 +3,8 @@ package vm
 import (
 	"encoding/binary"
 	"fmt"
+	"net/url"
+	"strings"
 
 	"github.com/tendermint/iavl"
 	"github.com/tendermint/tendermint/crypto/merkle"
@@ -169,6 +171,8 @@ func (c *iavlMerkleProofValidateBohr) Run(input []byte) (result []byte, err erro
 		multiStoreOpVerifier,
 		forbiddenSimpleValueOpVerifier,
 	}
+	c.basicIavlMerkleProofValidate.keyVerifier = keyVerifier
+	c.basicIavlMerkleProofValidate.opsVerifier = proofOpsVerifier
 	return c.basicIavlMerkleProofValidate.Run(input)
 }
 
@@ -179,6 +183,8 @@ func successfulMerkleResult() []byte {
 }
 
 type basicIavlMerkleProofValidate struct {
+	keyVerifier  lightclient.KeyVerifier
+	opsVerifier  merkle.ProofOpsVerifier
 	verifiers    []merkle.ProofOpVerifier
 	proofRuntime *merkle.ProofRuntime
 }
@@ -209,6 +215,9 @@ func (c *basicIavlMerkleProofValidate) Run(input []byte) (result []byte, err err
 		kvmp.SetProofRuntime(c.proofRuntime)
 	}
 	kvmp.SetVerifiers(c.verifiers)
+	kvmp.SetOpsVerifier(c.opsVerifier)
+	kvmp.SetKeyVerifier(c.keyVerifier)
+
 	valid := kvmp.Validate()
 	if !valid {
 		return nil, fmt.Errorf("invalid merkle proof")
@@ -270,3 +279,46 @@ func singleValueOpVerifier(op merkle.ProofOperator) error {
 	}
 	return nil
 }
+
+func proofOpsVerifier(poz merkle.ProofOperators) error {
+	if len(poz) != 2 {
+		return cmn.NewError("proof ops should be 2")
+	}
+
+	// for legacy proof type
+	if _, ok := poz[1].(lightclient.MultiStoreProofOp); ok {
+		if _, ok := poz[0].(iavl.IAVLValueOp); !ok {
+			return cmn.NewError("invalid proof op")
+		}
+		return nil
+	}
+
+	// for ics23 proof type
+	if op2, ok := poz[1].(lightclient.CommitmentOp); ok {
+		if op2.Type != lightclient.ProofOpSimpleMerkleCommitment {
+			return cmn.NewError("invalid proof op")
+		}
+
+		op1, ok := poz[0].(lightclient.CommitmentOp)
+		if !ok {
+			return cmn.NewError("invalid proof op")
+		}
+
+		if op1.Type != lightclient.ProofOpIAVLCommitment {
+			return cmn.NewError("invalid proof op")
+		}
+		return nil
+	}
+
+	return cmn.NewError("invalid proof type")
+}
+
+func keyVerifier(key string) error {
+	// https://github.com/bnb-chain/tendermint/blob/72375a6f3d4a72831cc65e73363db89a0073db38/crypto/merkle/proof_key_path.go#L88
+	// since the upper function is ambiguous, `x:00` can be decoded to both kind of key type
+	// we check the key here to make sure the key will not start from `x:`
+	if strings.HasPrefix(url.PathEscape(key), "x:") {
+		return cmn.NewError("key should not start with x:")
+	}
+	return nil
+}

core/vm/contracts_lightclient_test.go

@@ -6,6 +6,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/tendermint/iavl"
+	"github.com/tendermint/tendermint/crypto/merkle"
+	cmn "github.com/tendermint/tendermint/libs/common"
 
 	"github.com/stretchr/testify/require"
 
@@ -233,3 +236,78 @@ func TestMultiStore(t *testing.T) {
 	err = multiStoreOpVerifier(badProof)
 	assert.Error(t, err, "duplicated store")
 }
+
+func TestProofOpsVerifier(t *testing.T) {
+	tests := []struct {
+		ops merkle.ProofOperators
+		err error
+	}{
+		{
+			merkle.ProofOperators{
+				iavl.IAVLValueOp{},
+			},
+			cmn.NewError("proof ops should be 2"),
+		},
+		{
+			merkle.ProofOperators{
+				lightclient.MultiStoreProofOp{},
+				lightclient.MultiStoreProofOp{},
+			},
+			cmn.NewError("invalid proof op"),
+		},
+		{
+			merkle.ProofOperators{
+				iavl.IAVLValueOp{},
+				lightclient.MultiStoreProofOp{},
+			},
+			nil,
+		},
+		{
+			merkle.ProofOperators{
+				lightclient.CommitmentOp{Type: lightclient.ProofOpIAVLCommitment},
+				lightclient.CommitmentOp{Type: lightclient.ProofOpSimpleMerkleCommitment},
+			},
+			nil,
+		},
+		{
+			merkle.ProofOperators{
+				lightclient.CommitmentOp{Type: lightclient.ProofOpSimpleMerkleCommitment},
+				lightclient.CommitmentOp{Type: lightclient.ProofOpSimpleMerkleCommitment},
+			},
+			cmn.NewError("invalid proof op"),
+		},
+		{
+			merkle.ProofOperators{
+				lightclient.MultiStoreProofOp{},
+				iavl.IAVLValueOp{},
+			},
+			cmn.NewError("invalid proof type"),
+		},
+	}
+
+	for _, testCase := range tests {
+		err := proofOpsVerifier(testCase.ops)
+		assert.Equal(t, err, testCase.err)
+	}
+}
+
+func TestKeyVerifier(t *testing.T) {
+	tests := []struct {
+		key string
+		err error
+	}{
+		{
+			"x:sdfdfdsd",
+			cmn.NewError("key should not start with x:"),
+		},
+		{
+			"sdfxdfxs",
+			nil,
+		},
+	}
+
+	for _, testCase := range tests {
+		err := keyVerifier(testCase.key)
+		assert.Equal(t, err, testCase.err)
+	}
+}

core/vm/lightclient/types.go

@@ -130,8 +130,8 @@ func (cs ConsensusState) EncodeConsensusState() ([]byte, error) {
 }
 
 func (cs *ConsensusState) ApplyHeader(header *Header) (bool, error) {
-	if uint64(header.Height) < cs.Height {
-		return false, fmt.Errorf("header height < consensus height (%d < %d)", header.Height, cs.Height)
+	if uint64(header.Height) <= cs.Height {
+		return false, fmt.Errorf("header height <= consensus height (%d <= %d)", header.Height, cs.Height)
 	}
 
 	if err := header.Validate(cs.ChainID); err != nil {
@@ -209,15 +209,19 @@ func DecodeHeader(input []byte) (*Header, error) {
 	return &header, nil
 }
 
+type KeyVerifier func(string) error
+
 type KeyValueMerkleProof struct {
 	Key       []byte
 	Value     []byte
 	StoreName string
 	AppHash   []byte
 	Proof     *merkle.Proof
 
-	verifiers    []merkle.ProofOpVerifier
-	proofRuntime *merkle.ProofRuntime
+	keyVerifier      KeyVerifier
+	proofOpsVerifier merkle.ProofOpsVerifier
+	verifiers        []merkle.ProofOpVerifier
+	proofRuntime     *merkle.ProofRuntime
 }
 
 func (kvmp *KeyValueMerkleProof) SetProofRuntime(prt *merkle.ProofRuntime) {
@@ -228,7 +232,24 @@ func (kvmp *KeyValueMerkleProof) SetVerifiers(verifiers []merkle.ProofOpVerifier
 	kvmp.verifiers = verifiers
 }
 
+func (kvmp *KeyValueMerkleProof) SetOpsVerifier(verifier merkle.ProofOpsVerifier) {
+	kvmp.proofOpsVerifier = verifier
+}
+
+func (kvmp *KeyValueMerkleProof) SetKeyVerifier(keyChecker KeyVerifier) {
+	kvmp.keyVerifier = keyChecker
+}
+
 func (kvmp *KeyValueMerkleProof) Validate() bool {
+	if kvmp.keyVerifier != nil {
+		if err := kvmp.keyVerifier(kvmp.StoreName); err != nil {
+			return false
+		}
+		if err := kvmp.keyVerifier(string(kvmp.Key)); err != nil {
+			return false
+		}
+	}
+
 	kp := merkle.KeyPath{}
 	kp = kp.AppendKey([]byte(kvmp.StoreName), merkle.KeyEncodingURL)
 	kp = kp.AppendKey(kvmp.Key, merkle.KeyEncodingURL)
@@ -238,7 +259,7 @@ func (kvmp *KeyValueMerkleProof) Validate() bool {
 		return err == nil
 	}
 
-	err := kvmp.proofRuntime.VerifyValue(kvmp.Proof, kvmp.AppHash, kp.String(), kvmp.Value, kvmp.verifiers...)
+	err := kvmp.proofRuntime.VerifyValue(kvmp.Proof, kvmp.AppHash, kp.String(), kvmp.Value, kvmp.proofOpsVerifier, kvmp.verifiers...)
 	return err == nil
 }
 

go.mod

@@ -48,7 +48,7 @@ require (
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/panjf2000/ants/v2 v2.4.5
 	github.com/peterh/liner v1.1.1-0.20190123174540-a2c9a5303de7
-	github.com/pkg/errors v0.9.1
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/tsdb v0.7.1
 	github.com/rjeczalik/notify v0.9.1
 	github.com/rs/cors v1.7.0
@@ -124,4 +124,4 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
-replace github.com/tendermint/tendermint => github.com/bnb-chain/tendermint v0.31.14
+replace github.com/tendermint/tendermint => github.com/bnb-chain/tendermint v0.31.15

go.sum

@@ -98,8 +98,8 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r
 github.com/bmizerany/pat v0.0.0-20170815010413-6226ea591a40/go.mod h1:8rLXio+WjiTceGBHIoTvn60HIbs7Hm7bcHjyrSqYB9c=
 github.com/bnb-chain/ics23 v0.1.0 h1:DvjGOts2FBfbxB48384CYD1LbcrfjThFz8kowY/7KxU=
 github.com/bnb-chain/ics23 v0.1.0/go.mod h1:cU6lTGolbbLFsGCgceNB2AzplH1xecLp6+KXvxM32nI=
-github.com/bnb-chain/tendermint v0.31.14 h1:PrO3Owceg0iJ9tSXUUC0yst2VmQLkigUZt+EwegyYLg=
-github.com/bnb-chain/tendermint v0.31.14/go.mod h1:cmt8HHmQUSVaWQ/hoTefRxsh5X3ERaM1zCUIR0DPbFU=
+github.com/bnb-chain/tendermint v0.31.15 h1:Xyn/Hifb/7X4E1zSuMdnZdMSoM2Fx6cZuKCNnqIxbNU=
+github.com/bnb-chain/tendermint v0.31.15/go.mod h1:cmt8HHmQUSVaWQ/hoTefRxsh5X3ERaM1zCUIR0DPbFU=
 github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
 github.com/btcsuite/btcd v0.20.1-beta h1:Ik4hyJqN8Jfyv3S4AGBOmyouMsYE3EdYODkMbQjwPGw=
 github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ=