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

store/internal: validate keys before calling ProofsFromMap #9235

Merged
merged 1 commit into from
May 2, 2021
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
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
  • Loading branch information
Cuong Manh Le authored and cuonglm committed May 1, 2021
commit e30934b7d10ceb200a682f031d935190b79e7cb1
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions store/internal/proofs/create.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package proofs

import (
"errors"
"fmt"
"sort"

Expand All @@ -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{
Expand All @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
23 changes: 23 additions & 0 deletions store/internal/proofs/create_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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))
})
}
}