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

fix: add gRPC nil/zero check in query #13352

Merged
merged 13 commits into from
Sep 29, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,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.

### Deprecated

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
28 changes: 28 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,8 +114,31 @@ 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
}

yihuang marked this conversation as resolved.
Show resolved Hide resolved

func TestProtoCodecUnmarshalLengthPrefixedChecks(t *testing.T) {
cdc := codec.NewProtoCodec(createTestInterfaceRegistry())

Expand Down