Skip to content

Commit

Permalink
store/internal: validate keys before calling ProofsFromMap
Browse files Browse the repository at this point in the history
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
1 parent 72873a0 commit e30934b
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 0 deletions.
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))
})
}
}

0 comments on commit e30934b

Please sign in to comment.