Skip to content

Commit

Permalink
sec: add proof ops check and key checker (#1333)
Browse files Browse the repository at this point in the history
  • Loading branch information
yutianwu authored Mar 6, 2023
1 parent 85d1237 commit a671641
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 12 deletions.
4 changes: 2 additions & 2 deletions accounts/abi/bind/bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2109,12 +2109,12 @@ func TestGolangBindings(t *testing.T) {
t.Fatalf("failed to replace binding test dependency to current source tree: %v\n%s", err, out)
}

replacer = exec.Command(gocmd, "mod", "edit", "-x", "-require", "github.com/tendermint/[email protected]", "-replace", "github.com/tendermint/tendermint=github.com/bnb-chain/[email protected].12") // Repo root
replacer = exec.Command(gocmd, "mod", "edit", "-x", "-require", "github.com/tendermint/[email protected]", "-replace", "github.com/tendermint/tendermint=github.com/bnb-chain/[email protected].15") // Repo root
replacer.Dir = pkg
if out, err := replacer.CombinedOutput(); err != nil {
t.Fatalf("failed to replace tendermint dependency to bnb-chain source: %v\n%s", err, out)
}
tidier := exec.Command(gocmd, "mod", "tidy", "-compat=1.17")
tidier := exec.Command(gocmd, "mod", "tidy", "-compat=1.19")
tidier.Dir = pkg
if out, err := tidier.CombinedOutput(); err != nil {
t.Fatalf("failed to tidy Go module file: %v\n%s", err, out)
Expand Down
52 changes: 52 additions & 0 deletions core/vm/contracts_lightclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package vm
import (
"encoding/binary"
"fmt"
"net/url"
"strings"

"github.com/tendermint/iavl"
"github.com/tendermint/tendermint/crypto/merkle"
Expand Down Expand Up @@ -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)
}

Expand All @@ -179,6 +183,8 @@ func successfulMerkleResult() []byte {
}

type basicIavlMerkleProofValidate struct {
keyVerifier lightclient.KeyVerifier
opsVerifier merkle.ProofOpsVerifier
verifiers []merkle.ProofOpVerifier
proofRuntime *merkle.ProofRuntime
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
78 changes: 78 additions & 0 deletions core/vm/contracts_lightclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
}
31 changes: 26 additions & 5 deletions core/vm/lightclient/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand All @@ -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
}

Expand Down
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ require (
github.com/aws/aws-sdk-go-v2/credentials v1.1.1
github.com/aws/aws-sdk-go-v2/service/route53 v1.1.1
github.com/bnb-chain/ics23 v0.1.0
github.com/btcsuite/btcd v0.20.1-beta // indirect
github.com/cespare/cp v0.1.0
github.com/cloudflare/cloudflare-go v0.14.0
github.com/consensys/gnark-crypto v0.4.1-0.20210426202927-39ac3d4b3f1f
Expand Down Expand Up @@ -48,7 +47,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
Expand Down Expand Up @@ -124,4 +123,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
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down

0 comments on commit a671641

Please sign in to comment.