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(simapp/v2): register extra gRPC gateway routes #22786

Merged
merged 2 commits into from
Dec 9, 2024
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
195 changes: 142 additions & 53 deletions server/v2/cometbft/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cometbft
import (
"context"
"fmt"
"strings"

abci "github.com/cometbft/cometbft/abci/types"
abciproto "github.com/cometbft/cometbft/api/cometbft/abci/v1"
Expand All @@ -24,6 +25,8 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
)

type appSimulator[T transaction.Tx] interface {
Expand All @@ -50,51 +53,6 @@ func gRPCServiceRegistrar[T transaction.Tx](
}
}

// CometBFTAutoCLIDescriptor is the auto-generated CLI descriptor for the CometBFT service
var CometBFTAutoCLIDescriptor = &autocliv1.ServiceCommandDescriptor{
Service: cmtv1beta1.Service_ServiceDesc.ServiceName,
RpcCommandOptions: []*autocliv1.RpcCommandOptions{
{
RpcMethod: "GetNodeInfo",
Use: "node-info",
Short: "Query the current node info",
},
{
RpcMethod: "GetSyncing",
Use: "syncing",
Short: "Query node syncing status",
},
{
RpcMethod: "GetLatestBlock",
Use: "block-latest",
Short: "Query for the latest committed block",
},
{
RpcMethod: "GetBlockByHeight",
Use: "block-by-height <height>",
Short: "Query for a committed block by height",
Long: "Query for a specific committed block using the CometBFT RPC `block_by_height` method",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "height"}},
},
{
RpcMethod: "GetLatestValidatorSet",
Use: "validator-set",
Alias: []string{"validator-set-latest", "comet-validator-set", "cometbft-validator-set", "tendermint-validator-set"},
Short: "Query for the latest validator set",
},
{
RpcMethod: "GetValidatorSetByHeight",
Use: "validator-set-by-height <height>",
Short: "Query for a validator set by height",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "height"}},
},
{
RpcMethod: "ABCIQuery",
Skip: true,
},
},
}

type txServer[T transaction.Tx] struct {
clientCtx client.Context
txCodec transaction.Codec[T]
Expand All @@ -112,8 +70,33 @@ func (t txServer[T]) GetBlockWithTxs(context.Context, *txtypes.GetBlockWithTxsRe
}

// GetTx implements tx.ServiceServer.
func (t txServer[T]) GetTx(context.Context, *txtypes.GetTxRequest) (*txtypes.GetTxResponse, error) {
return nil, status.Error(codes.Unimplemented, "not implemented")
func (t txServer[T]) GetTx(ctx context.Context, req *txtypes.GetTxRequest) (*txtypes.GetTxResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "request cannot be nil")
}

if len(req.Hash) == 0 {
return nil, status.Error(codes.InvalidArgument, "tx hash cannot be empty")
}

result, err := authtx.QueryTx(t.clientCtx, req.Hash)
if err != nil {
if strings.Contains(err.Error(), "not found") {
return nil, status.Errorf(codes.NotFound, "tx not found: %s", req.Hash)
}

return nil, err
}

protoTx, ok := result.Tx.GetCachedValue().(*txtypes.Tx)
if !ok {
return nil, status.Errorf(codes.Internal, "expected %T, got %T", txtypes.Tx{}, result.Tx.GetCachedValue())
}

return &txtypes.GetTxResponse{
Tx: protoTx,
TxResponse: result,
}, nil
}

// GetTxsEvent implements tx.ServiceServer.
Expand Down Expand Up @@ -181,18 +164,79 @@ func (t txServer[T]) TxDecode(context.Context, *txtypes.TxDecodeRequest) (*txtyp
}

// TxDecodeAmino implements tx.ServiceServer.
func (t txServer[T]) TxDecodeAmino(context.Context, *txtypes.TxDecodeAminoRequest) (*txtypes.TxDecodeAminoResponse, error) {
return nil, status.Error(codes.Unimplemented, "not implemented")
func (t txServer[T]) TxDecodeAmino(_ context.Context, req *txtypes.TxDecodeAminoRequest) (*txtypes.TxDecodeAminoResponse, error) {
if req.AminoBinary == nil {
return nil, status.Error(codes.InvalidArgument, "invalid empty tx bytes")
}

var stdTx legacytx.StdTx
err := t.clientCtx.LegacyAmino.Unmarshal(req.AminoBinary, &stdTx)
if err != nil {
return nil, err
}

res, err := t.clientCtx.LegacyAmino.MarshalJSON(stdTx)
if err != nil {
return nil, err
}

return &txtypes.TxDecodeAminoResponse{
AminoJson: string(res),
}, nil
Comment on lines +167 to +185
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Add size limits to protect against memory exhaustion attacks

The gRPC service has default message size limits (10MB for receive and MaxInt32 for send), but the Amino codec operations lack specific size limits. To prevent potential memory exhaustion attacks:

  • Add size validation before unmarshaling in TxDecodeAmino (e.g., check if len(req.AminoBinary) exceeds a reasonable limit)
  • Add size validation for the JSON string in TxEncodeAmino (e.g., check if len(req.AminoJson) exceeds a reasonable limit)
  • Consider using the gRPC service's DefaultGRPCMaxRecvMsgSize (10MB) as a reference point for the size limits
🔗 Analysis chain

Consider adding size limits for Amino encoding/decoding.

The methods handle input validation but lack size limits for the binary/JSON data. This could potentially lead to memory exhaustion attacks.

Run the following script to check if size limits are implemented elsewhere:

Also applies to: 221-239


Let me gather more information about size limits in the codebase.


Let me check for any size validation in the Amino codec implementation.


Let me check if there are any size limits defined in the configuration or constants.


Let me check for any size limits in the gRPC configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for size limit implementations in Amino codec usage
rg -A 5 "LegacyAmino.*Marshal" 

Length of output: 9168


Script:

#!/bin/bash
# Check for any size limits or validation in the Amino codec implementation
rg -A 5 "func.*Unmarshal.*Amino" 

# Check for any configuration or constants related to size limits
rg -i "maxsize|max_size|size.*limit" 

Length of output: 95385


Script:

#!/bin/bash
# Check the Amino codec implementation for size validation
rg -A 5 "func.*Unmarshal.*" codec/amino.go

# Check for any size validation in the tx service
rg -A 5 "func.*TxDecode" server/v2/cometbft/grpc.go

Length of output: 1849


Script:

#!/bin/bash
# Check for any size limits in configuration files
rg -i "max.*size.*=|size.*limit.*=" 

# Check for size-related constants
rg "const.*size|const.*limit" 

Length of output: 117538


Script:

#!/bin/bash
# Check for gRPC size limits in configuration
rg -A 5 "DefaultGRPCMaxRecvMsgSize|DefaultGRPCMaxSendMsgSize" server/config/

# Check for any size validation in the tx service implementation
rg -A 5 "func.*Tx.*Amino" server/v2/cometbft/

Length of output: 2089

}

// TxEncode implements tx.ServiceServer.
func (t txServer[T]) TxEncode(context.Context, *txtypes.TxEncodeRequest) (*txtypes.TxEncodeResponse, error) {
return nil, status.Error(codes.Unimplemented, "not implemented")
func (t txServer[T]) TxEncode(_ context.Context, req *txtypes.TxEncodeRequest) (*txtypes.TxEncodeResponse, error) {
if req.Tx == nil {
return nil, status.Error(codes.InvalidArgument, "invalid empty tx")
}

bodyBytes, err := t.clientCtx.Codec.Marshal(req.Tx.Body)
if err != nil {
return nil, err
}

authInfoBytes, err := t.clientCtx.Codec.Marshal(req.Tx.AuthInfo)
if err != nil {
return nil, err
}

raw := &txtypes.TxRaw{
BodyBytes: bodyBytes,
AuthInfoBytes: authInfoBytes,
Signatures: req.Tx.Signatures,
}

encodedBytes, err := t.clientCtx.Codec.Marshal(raw)
if err != nil {
return nil, err
}

return &txtypes.TxEncodeResponse{
TxBytes: encodedBytes,
}, nil
}

// TxEncodeAmino implements tx.ServiceServer.
func (t txServer[T]) TxEncodeAmino(context.Context, *txtypes.TxEncodeAminoRequest) (*txtypes.TxEncodeAminoResponse, error) {
return nil, status.Error(codes.Unimplemented, "not implemented")
func (t txServer[T]) TxEncodeAmino(_ context.Context, req *txtypes.TxEncodeAminoRequest) (*txtypes.TxEncodeAminoResponse, error) {
if req.AminoJson == "" {
return nil, status.Error(codes.InvalidArgument, "invalid empty tx json")
}

var stdTx legacytx.StdTx
err := t.clientCtx.LegacyAmino.UnmarshalJSON([]byte(req.AminoJson), &stdTx)
if err != nil {
return nil, err
}

encodedBytes, err := t.clientCtx.LegacyAmino.Marshal(stdTx)
if err != nil {
return nil, err
}

return &txtypes.TxEncodeAminoResponse{
AminoBinary: encodedBytes,
}, nil
}

var _ txtypes.ServiceServer = txServer[transaction.Tx]{}
Expand Down Expand Up @@ -236,3 +280,48 @@ func (s nodeServer[T]) Status(ctx context.Context, _ *nodeservice.StatusRequest)
ValidatorHash: nodeInfo.LastBlockAppHash,
}, nil
}

// CometBFTAutoCLIDescriptor is the auto-generated CLI descriptor for the CometBFT service
var CometBFTAutoCLIDescriptor = &autocliv1.ServiceCommandDescriptor{
Service: cmtv1beta1.Service_ServiceDesc.ServiceName,
RpcCommandOptions: []*autocliv1.RpcCommandOptions{
{
RpcMethod: "GetNodeInfo",
Use: "node-info",
Short: "Query the current node info",
},
{
RpcMethod: "GetSyncing",
Use: "syncing",
Short: "Query node syncing status",
},
{
RpcMethod: "GetLatestBlock",
Use: "block-latest",
Short: "Query for the latest committed block",
},
{
RpcMethod: "GetBlockByHeight",
Use: "block-by-height <height>",
Short: "Query for a committed block by height",
Long: "Query for a specific committed block using the CometBFT RPC `block_by_height` method",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "height"}},
},
{
RpcMethod: "GetLatestValidatorSet",
Use: "validator-set",
Alias: []string{"validator-set-latest", "comet-validator-set", "cometbft-validator-set", "tendermint-validator-set"},
Short: "Query for the latest validator set",
},
{
RpcMethod: "GetValidatorSetByHeight",
Use: "validator-set-by-height <height>",
Short: "Query for a validator set by height",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "height"}},
},
{
RpcMethod: "ABCIQuery",
Skip: true,
},
},
}
31 changes: 24 additions & 7 deletions simapp/v2/simdv2/cmd/commands.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"io"

"github.com/spf13/cobra"
Expand All @@ -23,11 +24,14 @@
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/config"
"github.com/cosmos/cosmos-sdk/client/debug"
"github.com/cosmos/cosmos-sdk/client/grpc/cmtservice"
nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node"
"github.com/cosmos/cosmos-sdk/client/keys"
"github.com/cosmos/cosmos-sdk/client/rpc"
sdktelemetry "github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/version"
authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli"
"github.com/cosmos/cosmos-sdk/x/genutil"
Expand Down Expand Up @@ -153,13 +157,7 @@
if err != nil {
return nil, err
}

for _, mod := range deps.ModuleManager.Modules() {
if gmod, ok := mod.(module.HasGRPCGateway); ok {
// TODO(@julienrbrt) https://github.com/cosmos/cosmos-sdk/pull/22701#pullrequestreview-2470651390
gmod.RegisterGRPCGatewayRoutes(deps.ClientContext, grpcgatewayServer.GRPCGatewayRouter)
}
}
registerGRPCGatewayRoutes[T](deps, grpcgatewayServer)

// wire server commands
return serverv2.AddCommands[T](
Expand Down Expand Up @@ -264,3 +262,22 @@
return nil
}
}

// registerGRPCGatewayRoutes registers the gRPC gateway routes for all modules and other components
// TODO(@julienrbrt): Eventually, this should removed and directly done within the grpcgateway.Server
// ref: https://github.com/cosmos/cosmos-sdk/pull/22701#pullrequestreview-2470651390
func registerGRPCGatewayRoutes[T transaction.Tx](
deps CommandDependencies[T],
server *grpcgateway.Server[T],
) {
// those are the extra services that the CometBFT server implements (server/v2/cometbft/grpc.go)
cmtservice.RegisterGRPCGatewayRoutes(deps.ClientContext, server.GRPCGatewayRouter)
_ = nodeservice.RegisterServiceHandlerClient(context.Background(), server.GRPCGatewayRouter, nodeservice.NewServiceClient(deps.ClientContext))
_ = txtypes.RegisterServiceHandlerClient(context.Background(), server.GRPCGatewayRouter, txtypes.NewServiceClient(deps.ClientContext))
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

for _, mod := range deps.ModuleManager.Modules() {
if gmod, ok := mod.(module.HasGRPCGateway); ok {
gmod.RegisterGRPCGatewayRoutes(deps.ClientContext, server.GRPCGatewayRouter)
}
}
Dismissed Show dismissed Hide dismissed
}
Loading