From e30934b7d10ceb200a682f031d935190b79e7cb1 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Fri, 30 Apr 2021 17:08:27 +0700 Subject: [PATCH] store/internal: validate keys before calling ProofsFromMap Otherwise, an empty key as input or present in data can cause a panic at runtime. Caught by oss-fuzz: https://oss-fuzz.com/testcase-detail/4647668077953024 Fixes #9233 --- CHANGELOG.md | 2 ++ store/internal/proofs/create.go | 17 +++++++++++++++++ store/internal/proofs/create_test.go | 23 +++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3228549a7c13..e85e50e71066 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] * [\#9205](https://github.com/cosmos/cosmos-sdk/pull/9205) Improve readability in `abci` handleQueryP2P +* [\#9235](https://github.com/cosmos/cosmos-sdk/pull/9235) CreateMembershipProof/CreateNonMembershipProof now returns an error +if input key is empty, or input data contains empty key. ### Features diff --git a/store/internal/proofs/create.go b/store/internal/proofs/create.go index 02d57e336fb2..63b6bf69b149 100644 --- a/store/internal/proofs/create.go +++ b/store/internal/proofs/create.go @@ -1,6 +1,7 @@ package proofs import ( + "errors" "fmt" "sort" @@ -9,6 +10,11 @@ import ( sdkmaps "github.com/cosmos/cosmos-sdk/store/internal/maps" ) +var ( + ErrEmptyKey = errors.New("key is empty") + ErrEmptyKeyInData = errors.New("data contains empty key") +) + // TendermintSpec constrains the format from ics23-tendermint (crypto/merkle SimpleProof) var TendermintSpec = &ics23.ProofSpec{ LeafSpec: &ics23.LeafOp{ @@ -31,6 +37,9 @@ CreateMembershipProof will produce a CommitmentProof that the given key (and que If the key doesn't exist in the tree, this will return an error. */ func CreateMembershipProof(data map[string][]byte, key []byte) (*ics23.CommitmentProof, error) { + if len(key) == 0 { + return nil, ErrEmptyKey + } exist, err := createExistenceProof(data, key) if err != nil { return nil, err @@ -48,6 +57,9 @@ CreateNonMembershipProof will produce a CommitmentProof that the given key doesn If the key exists in the tree, this will return an error. */ func CreateNonMembershipProof(data map[string][]byte, key []byte) (*ics23.CommitmentProof, error) { + if len(key) == 0 { + return nil, ErrEmptyKey + } // ensure this key is not in the store if _, ok := data[string(key)]; ok { return nil, fmt.Errorf("cannot create non-membership proof if key is in map") @@ -89,6 +101,11 @@ func CreateNonMembershipProof(data map[string][]byte, key []byte) (*ics23.Commit } func createExistenceProof(data map[string][]byte, key []byte) (*ics23.ExistenceProof, error) { + for k := range data { + if k == "" { + return nil, ErrEmptyKeyInData + } + } value, ok := data[string(key)] if !ok { return nil, fmt.Errorf("cannot make existence proof if key is not in map") diff --git a/store/internal/proofs/create_test.go b/store/internal/proofs/create_test.go index c889c35386dc..dc95ee3d9697 100644 --- a/store/internal/proofs/create_test.go +++ b/store/internal/proofs/create_test.go @@ -1,9 +1,11 @@ package proofs import ( + "errors" "testing" ics23 "github.com/confio/ics23/go" + "github.com/stretchr/testify/assert" ) func TestCreateMembership(t *testing.T) { @@ -87,3 +89,24 @@ func TestCreateNonMembership(t *testing.T) { }) } } + +func TestInvalidKey(t *testing.T) { + tests := []struct { + name string + f func(data map[string][]byte, key []byte) (*ics23.CommitmentProof, error) + data map[string][]byte + key []byte + err error + }{ + {"CreateMembershipProof empty key", CreateMembershipProof, map[string][]byte{"": nil}, []byte(""), ErrEmptyKey}, + {"CreateMembershipProof empty key in data", CreateMembershipProof, map[string][]byte{"": nil, " ": nil}, []byte(" "), ErrEmptyKeyInData}, + {"CreateNonMembershipProof empty key", CreateNonMembershipProof, map[string][]byte{" ": nil}, []byte(""), ErrEmptyKey}, + {"CreateNonMembershipProof empty key in data", CreateNonMembershipProof, map[string][]byte{"": nil}, []byte(" "), ErrEmptyKeyInData}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := tc.f(tc.data, tc.key) + assert.True(t, errors.Is(err, tc.err)) + }) + } +}