baseapp, client: reject gRPC connections with out-of-range/nefarious …

…x-cosmos-block-height values (cosmos#7663)

* baseapp, client: reject gRPC connections with out-of-range/nefarious x-cosmos-block-height values

Rejects gRPC connections that send out-of-range x-cosmos-block-height
values that previously weren't checked for. We now reject any negative
values and any value greater than max(int64) aka >9223372036854775807.

Also added an enforcement for returning an error if any negative heights
are passed into (*BaseApp).createQueryContext.

Fixes cosmos#7662

* baseapp, client: reject gRPC connections with out-of-range/nefarious x-cosmos-block-height values

Rejects gRPC connections that send out-of-range x-cosmos-block-height
values that previously weren't checked for. We now reject any negative
values and any value greater than max(int64) aka >9223372036854775807.

Also added an enforcement for returning an error if any negative heights
are passed into (*BaseApp).createQueryContext.

Fixes cosmos#7662

* Address Robert's feedback to extract negative height checker

* Fix tests

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

baseapp/abci.go

@@ -537,9 +537,24 @@ func gRPCErrorToSDKError(err error) error {
 	}
 }
 
+func checkNegativeHeight(height int64) error {
+	if height < 0 {
+		// Reject invalid heights.
+		return sdkerrors.Wrap(
+			sdkerrors.ErrInvalidRequest,
+			"cannot query with height < 0; please provide a valid height",
+		)
+	}
+	return nil
+}
+
 // createQueryContext creates a new sdk.Context for a query, taking as args
 // the block height and whether the query needs a proof or not.
 func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, error) {
+	if err := checkNegativeHeight(height); err != nil {
+		return sdk.Context{}, err
+	}
+
 	// when a client did not provide a query height, manually inject the latest
 	if height == 0 {
 		height = app.LastBlockHeight()

baseapp/abci_test.go

@@ -1,6 +1,7 @@
 package baseapp
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -116,3 +117,25 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		})
 	}
 }
+
+// Test and ensure that negative heights always cause errors.
+// See issue https://github.com/cosmos/cosmos-sdk/issues/7662.
+func TestBaseAppCreateQueryContextRejectsNegativeHeights(t *testing.T) {
+	t.Parallel()
+
+	logger := defaultLogger()
+	db := dbm.NewMemDB()
+	name := t.Name()
+	app := NewBaseApp(name, logger, db, nil)
+
+	proves := []bool{
+		false, true,
+	}
+	for _, prove := range proves {
+		t.Run(fmt.Sprintf("prove=%t", prove), func(t *testing.T) {
+			sctx, err := app.createQueryContext(-10, true)
+			require.Error(t, err)
+			require.Equal(t, sctx, sdk.Context{})
+		})
+	}
+}

baseapp/grpcserver.go

@@ -11,6 +11,7 @@ import (
 	"google.golang.org/grpc/status"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	grpctypes "github.com/cosmos/cosmos-sdk/types/grpc"
 )
 
@@ -33,6 +34,11 @@ func (app *BaseApp) RegisterGRPCServer(server gogogrpc.Server) {
 		if heightHeaders := md.Get(grpctypes.GRPCBlockHeightHeader); len(heightHeaders) > 0 {
 			height, err = strconv.ParseInt(heightHeaders[0], 10, 64)
 			if err != nil {
+				return nil, sdkerrors.Wrapf(
+					sdkerrors.ErrInvalidRequest,
+					"Baseapp.RegisterGRPCServer: invalid height header %q: %v", grpctypes.GRPCBlockHeightHeader, err)
+			}
+			if err := checkNegativeHeight(height); err != nil {
 				return nil, err
 			}
 		}

client/grpc_query.go

@@ -15,6 +15,7 @@ import (
 	grpctypes "github.com/cosmos/cosmos-sdk/types/grpc"
 
 	"github.com/cosmos/cosmos-sdk/codec/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 var _ gogogrpc.ClientConn = Context{}
@@ -35,6 +36,11 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, args, reply
 		if err != nil {
 			return err
 		}
+		if height < 0 {
+			return sdkerrors.Wrapf(
+				sdkerrors.ErrInvalidRequest,
+				"client.Context.Invoke: height (%d) from %q must be >= 0", height, grpctypes.GRPCBlockHeightHeader)
+		}
 
 		ctx = ctx.WithHeight(height)
 	}

server/grpc/server_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/metadata"
@@ -48,6 +49,7 @@ func (s *IntegrationTestSuite) TestGRPCServer() {
 		grpc.WithInsecure(), // Or else we get "no transport security set"
 	)
 	s.Require().NoError(err)
+	defer conn.Close()
 
 	// gRPC query to test service should work
 	testClient := testdata.NewQueryClient(conn)
@@ -99,6 +101,42 @@ func (s *IntegrationTestSuite) TestGRPCServer() {
 	s.Require().True(servicesMap["cosmos.bank.v1beta1.Query"])
 }
 
+// Test and enforce that we upfront reject any connections to baseapp containing
+// invalid initial x-cosmos-block-height that aren't positive  and in the range [0, max(int64)]
+// See issue https://github.com/cosmos/cosmos-sdk/issues/7662.
+func (s *IntegrationTestSuite) TestGRPCServerInvalidHeaderHeights() {
+	t := s.T()
+	val0 := s.network.Validators[0]
+
+	// We should reject connections with invalid block heights off the bat.
+	invalidHeightStrs := []struct {
+		value   string
+		wantErr string
+	}{
+		{"-1", "height < 0"},
+		{"9223372036854775808", "value out of range"}, // > max(int64) by 1
+		{"-10", "height < 0"},
+		{"18446744073709551615", "value out of range"}, // max uint64, which is  > max(int64)
+		{"-9223372036854775809", "value out of range"}, // Out of the range of for negative int64
+	}
+	for _, tt := range invalidHeightStrs {
+		t.Run(tt.value, func(t *testing.T) {
+			conn, err := grpc.Dial(
+				val0.AppConfig.GRPC.Address,
+				grpc.WithInsecure(), // Or else we get "no transport security set"
+			)
+			defer conn.Close()
+
+			testClient := testdata.NewQueryClient(conn)
+			ctx := metadata.AppendToOutgoingContext(context.Background(), grpctypes.GRPCBlockHeightHeader, tt.value)
+			testRes, err := testClient.Echo(ctx, &testdata.EchoRequest{Message: "hello"})
+			require.Error(t, err)
+			require.Nil(t, testRes)
+			require.Contains(t, err.Error(), tt.wantErr)
+		})
+	}
+}
+
 func TestIntegrationTestSuite(t *testing.T) {
 	suite.Run(t, new(IntegrationTestSuite))
 }