diff --git a/x/wasm/keeper/keeper.go b/x/wasm/keeper/keeper.go index e5e65023f0..3112a26eb5 100644 --- a/x/wasm/keeper/keeper.go +++ b/x/wasm/keeper/keeper.go @@ -119,7 +119,7 @@ func NewKeeper( capabilityKeeper types.CapabilityKeeper, portSource types.ICS20TransferPortSource, router MessageRouter, - queryRouter GRPCQueryRouter, + _ GRPCQueryRouter, homeDir string, wasmConfig types.WasmConfig, availableCapabilities string, @@ -150,7 +150,7 @@ func NewKeeper( maxQueryStackSize: types.DefaultMaxQueryStackSize, acceptedAccountTypes: defaultAcceptedAccountTypes, } - keeper.wasmVMQueryHandler = DefaultQueryPlugins(bankKeeper, stakingKeeper, distKeeper, channelKeeper, queryRouter, keeper, cdc) + keeper.wasmVMQueryHandler = DefaultQueryPlugins(bankKeeper, stakingKeeper, distKeeper, channelKeeper, keeper) for _, o := range opts { o.apply(keeper) } diff --git a/x/wasm/keeper/options.go b/x/wasm/keeper/options.go index cc5547f22c..223a771f63 100644 --- a/x/wasm/keeper/options.go +++ b/x/wasm/keeper/options.go @@ -57,7 +57,7 @@ func WithQueryHandlerDecorator(d func(old WasmVMQueryHandler) WasmVMQueryHandler } // WithQueryPlugins is an optional constructor parameter to pass custom query plugins for wasmVM requests. -// This option expects the default `QueryHandler` set an should not be combined with Option `WithQueryHandler` or `WithQueryHandlerDecorator`. +// This option expects the default `QueryHandler` set and should not be combined with Option `WithQueryHandler` or `WithQueryHandlerDecorator`. func WithQueryPlugins(x *QueryPlugins) Option { return optsFn(func(k *Keeper) { q, ok := k.wasmVMQueryHandler.(QueryPlugins) diff --git a/x/wasm/keeper/query_plugins.go b/x/wasm/keeper/query_plugins.go index da312ba675..7196860d90 100644 --- a/x/wasm/keeper/query_plugins.go +++ b/x/wasm/keeper/query_plugins.go @@ -102,16 +102,14 @@ func DefaultQueryPlugins( staking types.StakingKeeper, distKeeper types.DistributionKeeper, channelKeeper types.ChannelKeeper, - queryRouter GRPCQueryRouter, wasm wasmQueryKeeper, - codec codec.Codec, ) QueryPlugins { return QueryPlugins{ Bank: BankQuerier(bank), Custom: NoCustomQuerier, IBC: IBCQuerier(wasm, channelKeeper), Staking: StakingQuerier(staking, distKeeper), - Stargate: StargateQuerier(queryRouter, codec), + Stargate: RejectStargateQuerier(), Wasm: WasmQuerier(wasm), } } @@ -282,10 +280,30 @@ func IBCQuerier(wasm contractMetaDataSource, channelKeeper types.ChannelKeeper) } } -func StargateQuerier(queryRouter GRPCQueryRouter, codec codec.Codec) func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { +// RejectStargateQuerier rejects all stargate queries +func RejectStargateQuerier() func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { return func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { - protoResponse, whitelisted := AcceptList.Load(request.Path) - if !whitelisted { + return nil, wasmvmtypes.UnsupportedRequest{Kind: "Stargate queries are disabled"} + } +} + +// AcceptedStargateQueries define accepted Stargate queries as a map with path as key and response type as value. +// For example: +// acceptList["/cosmos.auth.v1beta1.Query/Account"]= &authtypes.QueryAccountResponse{} +type AcceptedStargateQueries map[string]codec.ProtoMarshaler + +// AcceptListStargateQuerier supports a preconfigured set of stargate queries only. +// All arguments must be non nil. +// +// Warning: Chains need to test and maintain their accept list carefully. +// There were critical consensus breaking issues in the past with non-deterministic behaviour in the SDK. +// +// This queries can be set via WithQueryPlugins option in the wasm keeper constructor: +// WithQueryPlugins(&QueryPlugins{Stargate: AcceptListStargateQuerier(acceptList, queryRouter, codec)}) +func AcceptListStargateQuerier(acceptList AcceptedStargateQueries, queryRouter GRPCQueryRouter, codec codec.Codec) func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { + return func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { + protoResponse, accepted := acceptList[request.Path] + if !accepted { return nil, wasmvmtypes.UnsupportedRequest{Kind: fmt.Sprintf("'%s' path is not allowed from the contract", request.Path)} } @@ -302,12 +320,7 @@ func StargateQuerier(queryRouter GRPCQueryRouter, codec codec.Codec) func(ctx sd return nil, err } - bz, err := ConvertProtoToJSONMarshal(protoResponse, res.Value, codec) - if err != nil { - return nil, err - } - - return bz, nil + return ConvertProtoToJSONMarshal(codec, protoResponse, res.Value) } } @@ -557,22 +570,16 @@ func ConvertSdkCoinToWasmCoin(coin sdk.Coin) wasmvmtypes.Coin { // ConvertProtoToJSONMarshal unmarshals the given bytes into a proto message and then marshals it to json. // This is done so that clients calling stargate queries do not need to define their own proto unmarshalers, // being able to use response directly by json marshalling, which is supported in cosmwasm. -func ConvertProtoToJSONMarshal(protoResponse interface{}, bz []byte, cdc codec.Codec) ([]byte, error) { - // all values are proto message - message, ok := protoResponse.(codec.ProtoMarshaler) - if !ok { - return nil, wasmvmtypes.Unknown{} - } - +func ConvertProtoToJSONMarshal(cdc codec.Codec, protoResponse codec.ProtoMarshaler, bz []byte) ([]byte, error) { // unmarshal binary into stargate response data structure - err := cdc.Unmarshal(bz, message) + err := cdc.Unmarshal(bz, protoResponse) if err != nil { - return nil, wasmvmtypes.Unknown{} + return nil, sdkerrors.Wrap(err, "to proto") } - bz, err = cdc.MarshalJSON(message) + bz, err = cdc.MarshalJSON(protoResponse) if err != nil { - return nil, wasmvmtypes.Unknown{} + return nil, sdkerrors.Wrap(err, "to json") } return bz, nil diff --git a/x/wasm/keeper/query_plugins_test.go b/x/wasm/keeper/query_plugins_test.go index 52440b25fb..d27cc98c30 100644 --- a/x/wasm/keeper/query_plugins_test.go +++ b/x/wasm/keeper/query_plugins_test.go @@ -5,25 +5,27 @@ import ( "encoding/json" "fmt" "testing" + "time" - "github.com/CosmWasm/wasmd/app" - "google.golang.org/protobuf/runtime/protoiface" - + wasmvmtypes "github.com/CosmWasm/wasmvm/types" + "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "github.com/cosmos/cosmos-sdk/store" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - "github.com/golang/protobuf/proto" - dbm "github.com/tendermint/tm-db" - - wasmvmtypes "github.com/CosmWasm/wasmvm/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/query" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + dbm "github.com/tendermint/tm-db" + "github.com/CosmWasm/wasmd/app" "github.com/CosmWasm/wasmd/x/wasm/keeper" "github.com/CosmWasm/wasmd/x/wasm/keeper/wasmtesting" "github.com/CosmWasm/wasmd/x/wasm/types" @@ -486,6 +488,71 @@ func TestQueryErrors(t *testing.T) { } } +func TestAcceptListStargateQuerier(t *testing.T) { + wasmApp := app.SetupWithEmptyStore(t) + ctx := wasmApp.NewUncachedContext(false, tmproto.Header{ChainID: "foo", Height: 1, Time: time.Now()}) + wasmApp.StakingKeeper.SetParams(ctx, stakingtypes.DefaultParams()) + + addrs := app.AddTestAddrs(wasmApp, ctx, 2, sdk.NewInt(1_000_000)) + accepted := keeper.AcceptedStargateQueries{ + "/cosmos.auth.v1beta1.Query/Account": &authtypes.QueryAccountResponse{}, + "/no/route/to/this": &authtypes.QueryAccountResponse{}, + } + + marshal := func(pb proto.Message) []byte { + b, err := proto.Marshal(pb) + require.NoError(t, err) + return b + } + + specs := map[string]struct { + req *wasmvmtypes.StargateQuery + expErr bool + expResp string + }{ + "in accept list - success result": { + req: &wasmvmtypes.StargateQuery{ + Path: "/cosmos.auth.v1beta1.Query/Account", + Data: marshal(&authtypes.QueryAccountRequest{Address: addrs[0].String()}), + }, + expResp: fmt.Sprintf(`{"account":{"@type":"/cosmos.auth.v1beta1.BaseAccount","address":%q,"pub_key":null,"account_number":"1","sequence":"0"}}`, addrs[0].String()), + }, + "in accept list - error result": { + req: &wasmvmtypes.StargateQuery{ + Path: "/cosmos.auth.v1beta1.Query/Account", + Data: marshal(&authtypes.QueryAccountRequest{Address: sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()).String()}), + }, + expErr: true, + }, + "not in accept list": { + req: &wasmvmtypes.StargateQuery{ + Path: "/cosmos.bank.v1beta1.Query/AllBalances", + Data: marshal(&banktypes.QueryAllBalancesRequest{Address: addrs[0].String()}), + }, + expErr: true, + }, + "unknown route": { + req: &wasmvmtypes.StargateQuery{ + Path: "/no/route/to/this", + Data: marshal(&banktypes.QueryAllBalancesRequest{Address: addrs[0].String()}), + }, + expErr: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + q := keeper.AcceptListStargateQuerier(accepted, wasmApp.GRPCQueryRouter(), wasmApp.AppCodec()) + gotBz, gotErr := q(ctx, spec.req) + if spec.expErr { + require.Error(t, gotErr) + return + } + require.NoError(t, gotErr) + assert.JSONEq(t, spec.expResp, string(gotBz), string(gotBz)) + }) + } +} + type mockWasmQueryKeeper struct { GetContractInfoFn func(ctx sdk.Context, contractAddress sdk.AccAddress) *types.ContractInfo QueryRawFn func(ctx sdk.Context, contractAddress sdk.AccAddress, key []byte) []byte @@ -548,13 +615,13 @@ func (m bankKeeperMock) GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk return m.GetAllBalancesFn(ctx, addr) } -func TestConvertProtoToJSONMarshal(t *testing.T) { +func TestCo3nvertProtoToJSONMarshal(t *testing.T) { testCases := []struct { name string queryPath string - protoResponseStruct proto.Message + protoResponseStruct codec.ProtoMarshaler originalResponse string - expectedProtoResponse proto.Message + expectedProtoResponse codec.ProtoMarshaler expectedError bool }{ { @@ -573,20 +640,18 @@ func TestConvertProtoToJSONMarshal(t *testing.T) { name: "invalid proto response struct", queryPath: "/cosmos.bank.v1beta1.Query/AllBalances", originalResponse: "0a090a036261721202333012050a03666f6f", - protoResponseStruct: protoiface.MessageV1(nil), + protoResponseStruct: &authtypes.QueryAccountResponse{}, expectedError: true, }, } for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - // set up app for testing - wasmApp := app.SetupWithEmptyStore(t) - originalVersionBz, err := hex.DecodeString(tc.originalResponse) require.NoError(t, err) + appCodec := app.MakeEncodingConfig().Marshaler - jsonMarshalledResponse, err := keeper.ConvertProtoToJSONMarshal(tc.protoResponseStruct, originalVersionBz, wasmApp.AppCodec()) + jsonMarshalledResponse, err := keeper.ConvertProtoToJSONMarshal(appCodec, tc.protoResponseStruct, originalVersionBz) if tc.expectedError { require.Error(t, err) return @@ -594,7 +659,7 @@ func TestConvertProtoToJSONMarshal(t *testing.T) { require.NoError(t, err) // check response by json marshalling proto response into json response manually - jsonMarshalExpectedResponse, err := wasmApp.AppCodec().MarshalJSON(tc.expectedProtoResponse) + jsonMarshalExpectedResponse, err := appCodec.MarshalJSON(tc.expectedProtoResponse) require.NoError(t, err) require.JSONEq(t, string(jsonMarshalledResponse), string(jsonMarshalExpectedResponse)) }) @@ -609,8 +674,8 @@ func TestDeterministicJsonMarshal(t *testing.T) { originalResponse string updatedResponse string queryPath string - responseProtoStruct interface{} - expectedProto func() proto.Message + responseProtoStruct codec.ProtoMarshaler + expectedProto func() codec.ProtoMarshaler }{ /** * @@ -638,7 +703,7 @@ func TestDeterministicJsonMarshal(t *testing.T) { "0a530a202f636f736d6f732e617574682e763162657461312e426173654163636f756e74122f0a2d636f736d6f733166387578756c746e3873717a687a6e72737a3371373778776171756867727367366a79766679122d636f736d6f733166387578756c746e3873717a687a6e72737a3371373778776171756867727367366a79766679", "/cosmos.auth.v1beta1.Query/Account", &authtypes.QueryAccountResponse{}, - func() proto.Message { + func() codec.ProtoMarshaler { account := authtypes.BaseAccount{ Address: "cosmos1f8uxultn8sqzhznrsz3q77xwaquhgrsg6jyvfy", } @@ -653,23 +718,23 @@ func TestDeterministicJsonMarshal(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - wasmApp := app.SetupWithEmptyStore(t) + appCodec := app.MakeEncodingConfig().Marshaler originVersionBz, err := hex.DecodeString(tc.originalResponse) require.NoError(t, err) - jsonMarshalledOriginalBz, err := keeper.ConvertProtoToJSONMarshal(tc.responseProtoStruct, originVersionBz, wasmApp.AppCodec()) + jsonMarshalledOriginalBz, err := keeper.ConvertProtoToJSONMarshal(appCodec, tc.responseProtoStruct, originVersionBz) require.NoError(t, err) newVersionBz, err := hex.DecodeString(tc.updatedResponse) require.NoError(t, err) - jsonMarshalledUpdatedBz, err := keeper.ConvertProtoToJSONMarshal(tc.responseProtoStruct, newVersionBz, wasmApp.AppCodec()) + jsonMarshalledUpdatedBz, err := keeper.ConvertProtoToJSONMarshal(appCodec, tc.responseProtoStruct, newVersionBz) require.NoError(t, err) // json marshalled bytes should be the same since we use the same proto struct for unmarshalling require.Equal(t, jsonMarshalledOriginalBz, jsonMarshalledUpdatedBz) // raw build also make same result - jsonMarshalExpectedResponse, err := wasmApp.AppCodec().MarshalJSON(tc.expectedProto()) + jsonMarshalExpectedResponse, err := appCodec.MarshalJSON(tc.expectedProto()) require.NoError(t, err) require.Equal(t, jsonMarshalledUpdatedBz, jsonMarshalExpectedResponse) }) diff --git a/x/wasm/keeper/stargate_whitelist.go b/x/wasm/keeper/stargate_whitelist.go deleted file mode 100644 index ce90738035..0000000000 --- a/x/wasm/keeper/stargate_whitelist.go +++ /dev/null @@ -1,18 +0,0 @@ -package keeper - -import ( - "sync" -) - -// AcceptList keeps whitelist and its deterministic -// response binding for stargate queries. -// -// The query can be multi-thread, so we have to use -// thread safe sync.Map. -var AcceptList sync.Map - -// Define AcceptList here as maps using 'AcceptList' -// e.x) AcceptList.Store("/cosmos.auth.v1beta1.Query/Account", &authtypes.QueryAccountResponse{}) -func init() { - -} diff --git a/x/wasm/simulation/proposals.go b/x/wasm/simulation/proposals.go index 8e73f04f64..1ee9584811 100644 --- a/x/wasm/simulation/proposals.go +++ b/x/wasm/simulation/proposals.go @@ -3,12 +3,13 @@ package simulation import ( "math/rand" - "github.com/CosmWasm/wasmd/app/params" - "github.com/CosmWasm/wasmd/x/wasm/keeper/testdata" - "github.com/CosmWasm/wasmd/x/wasm/types" sdk "github.com/cosmos/cosmos-sdk/types" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/simulation" + + "github.com/CosmWasm/wasmd/app/params" + "github.com/CosmWasm/wasmd/x/wasm/keeper/testdata" + "github.com/CosmWasm/wasmd/x/wasm/types" ) const (