Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sec: add proof ops check and key checker #1333

Merged
merged 8 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:") {
unclezoro marked this conversation as resolved.
Show resolved Hide resolved
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)
unclezoro marked this conversation as resolved.
Show resolved Hide resolved
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