Skip to content

Commit

Permalink
fix: add gRPC nil/zero check in query (#13352)
Browse files Browse the repository at this point in the history
  • Loading branch information
yihuang authored Sep 29, 2022
1 parent 3d0e214 commit a9f02d9
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/gov) [#13051](https://github.com/cosmos/cosmos-sdk/pull/13051) In SubmitPropsal, when a legacy msg fails it's handler call, wrap the error as ErrInvalidProposalContent (instead of ErrNoProposalHandlerExists).
* (x/gov) [#13045](https://github.com/cosmos/cosmos-sdk/pull/13045) Fix gov migrations for v3(0.46).
* (Store) [#13334](https://github.com/cosmos/cosmos-sdk/pull/13334) Call streaming listeners for deliver tx event, it was removed accidentally.
* (grpc) [#13352](https://github.com/cosmos/cosmos-sdk/pull/13352) fix grpc query panic that could crash the node.
* (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19.

### Deprecated
Expand Down
12 changes: 6 additions & 6 deletions baseapp/grpcrouter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func TestGRPCQueryRouter(t *testing.T) {
require.NotNil(t, res)
require.Equal(t, "hello", res.Message)

require.Panics(t, func() {
_, _ = client.Echo(context.Background(), nil)
})
res, err = client.Echo(context.Background(), nil)
require.Nil(t, err)
require.Empty(t, res.Message)

res2, err := client.SayHello(context.Background(), &testdata.SayHelloRequest{Name: "Foo"})
require.Nil(t, err)
Expand Down Expand Up @@ -153,9 +153,9 @@ func testQueryDataRacesSameHandler(t *testing.T, makeClientConn func(*baseapp.GR
require.NotNil(t, res)
require.Equal(t, "hello", res.Message)

require.Panics(t, func() {
_, _ = client.Echo(context.Background(), nil)
})
res, err = client.Echo(context.Background(), nil)
require.Nil(t, err)
require.Empty(t, res.Message)

res2, err := client.SayHello(context.Background(), &testdata.SayHelloRequest{Name: "Foo"})
require.Nil(t, err)
Expand Down
5 changes: 5 additions & 0 deletions codec/proto_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ func NewProtoCodec(interfaceRegistry types.InterfaceRegistry) *ProtoCodec {
// NOTE: this function must be used with a concrete type which
// implements proto.Message. For interface please use the codec.MarshalInterface
func (pc *ProtoCodec) Marshal(o ProtoMarshaler) ([]byte, error) {
// Size() check can catch the typed nil value.
if o == nil || o.Size() == 0 {
// return empty bytes instead of nil, because nil has special meaning in places like store.Set
return []byte{}, nil
}
return o.Marshal()
}

Expand Down
27 changes: 27 additions & 0 deletions codec/proto_codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@ package codec_test
import (
"errors"
"fmt"
"math"
"reflect"
"testing"

"github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/encoding"
"google.golang.org/grpc/status"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
)

func createTestInterfaceRegistry() types.InterfaceRegistry {
Expand Down Expand Up @@ -109,6 +114,28 @@ func TestProtoCodecMarshal(t *testing.T) {

err = cartoonCdc.UnmarshalInterface(bz, &cartoon)
require.NoError(t, err)

// test typed nil input shouldn't panic
var v *banktypes.QueryBalanceResponse
bz, err = grpcServerEncode(cartoonCdc.GRPCCodec(), v)
require.NoError(t, err)
require.Empty(t, bz)
}

// Emulate grpc server implementation
// https://github.com/grpc/grpc-go/blob/b1d7f56b81b7902d871111b82dec6ba45f854ede/rpc_util.go#L590
func grpcServerEncode(c encoding.Codec, msg interface{}) ([]byte, error) {
if msg == nil { // NOTE: typed nils will not be caught by this check
return nil, nil
}
b, err := c.Marshal(msg)
if err != nil {
return nil, status.Errorf(codes.Internal, "grpc: error while marshaling: %v", err.Error())
}
if uint(len(b)) > math.MaxUint32 {
return nil, status.Errorf(codes.ResourceExhausted, "grpc: message too large (%d bytes)", len(b))
}
return b, nil
}

func TestProtoCodecUnmarshalLengthPrefixedChecks(t *testing.T) {
Expand Down

0 comments on commit a9f02d9

Please sign in to comment.