From b2d87668e0e4d57aba91e03571a8cfbc1d56d099 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Mon, 13 Sep 2021 11:30:43 +0200 Subject: [PATCH 1/2] Ensure query isolation --- x/wasm/keeper/keeper_test.go | 31 +++++++++++++++++++++++++++++++ x/wasm/keeper/query_plugins.go | 8 ++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/x/wasm/keeper/keeper_test.go b/x/wasm/keeper/keeper_test.go index 4a43059b33..835d10aa96 100644 --- a/x/wasm/keeper/keeper_test.go +++ b/x/wasm/keeper/keeper_test.go @@ -1652,6 +1652,37 @@ func TestReply(t *testing.T) { } } +func TestQueryIsolation(t *testing.T) { + ctx, keepers := CreateTestInput(t, false, SupportedFeatures) + k := keepers.WasmKeeper + var mock wasmtesting.MockWasmer + wasmtesting.MakeInstantiable(&mock) + example := SeedNewContractInstance(t, ctx, keepers, &mock) + WithQueryHandlerDecorator(func(other WasmVMQueryHandler) WasmVMQueryHandler { + return WasmVMQueryHandlerFn(func(ctx sdk.Context, caller sdk.AccAddress, request wasmvmtypes.QueryRequest) ([]byte, error) { + if request.Custom == nil { + return other.HandleQuery(ctx, caller, request) + } + // here we write to DB which should not be persisted + ctx.KVStore(k.storeKey).Set([]byte(`set_in_query`), []byte(`this_is_allowed`)) + return nil, nil + }) + }).apply(k) + + // when + mock.ReplyFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, reply wasmvmtypes.Reply, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) { + _, err := querier.Query(wasmvmtypes.QueryRequest{ + Custom: []byte(`{}`), + }, 1_000_000) + require.NoError(t, err) + return &wasmvmtypes.Response{}, 0, nil + } + em := sdk.NewEventManager() + _, gotErr := k.reply(ctx.WithEventManager(em), example.Contract, wasmvmtypes.Reply{}) + require.NoError(t, gotErr) + assert.Nil(t, ctx.KVStore(k.storeKey).Get([]byte(`set_in_query`))) +} + func TestBuildContractAddress(t *testing.T) { specs := map[string]struct { srcCodeID uint64 diff --git a/x/wasm/keeper/query_plugins.go b/x/wasm/keeper/query_plugins.go index b767cd999a..5866a23989 100644 --- a/x/wasm/keeper/query_plugins.go +++ b/x/wasm/keeper/query_plugins.go @@ -46,15 +46,15 @@ type GRPCQueryRouter interface { var _ wasmvmtypes.Querier = QueryHandler{} func (q QueryHandler) Query(request wasmvmtypes.QueryRequest, gasLimit uint64) ([]byte, error) { - // set a limit for a subctx + // set a limit for a subCtx sdkGas := q.gasRegister.FromWasmVMGas(gasLimit) - subctx := q.Ctx.WithGasMeter(sdk.NewGasMeter(sdkGas)) + subCtx, _ := q.Ctx.WithGasMeter(sdk.NewGasMeter(sdkGas)).CacheContext() // make sure we charge the higher level context even on panic defer func() { - q.Ctx.GasMeter().ConsumeGas(subctx.GasMeter().GasConsumed(), "contract sub-query") + q.Ctx.GasMeter().ConsumeGas(subCtx.GasMeter().GasConsumed(), "contract sub-query") }() - return q.Plugins.HandleQuery(subctx, q.Caller, request) + return q.Plugins.HandleQuery(subCtx, q.Caller, request) } func (q QueryHandler) GasConsumed() uint64 { From 0f4c8b527e266c5184965418e1aab18e650c77b1 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Tue, 14 Sep 2021 17:16:31 +0200 Subject: [PATCH 2/2] Review feedback --- x/wasm/keeper/query_plugins.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/wasm/keeper/query_plugins.go b/x/wasm/keeper/query_plugins.go index 5866a23989..2ac014d63b 100644 --- a/x/wasm/keeper/query_plugins.go +++ b/x/wasm/keeper/query_plugins.go @@ -48,6 +48,7 @@ var _ wasmvmtypes.Querier = QueryHandler{} func (q QueryHandler) Query(request wasmvmtypes.QueryRequest, gasLimit uint64) ([]byte, error) { // set a limit for a subCtx sdkGas := q.gasRegister.FromWasmVMGas(gasLimit) + // discard all changes/ events in subCtx by not committing the cached context subCtx, _ := q.Ctx.WithGasMeter(sdk.NewGasMeter(sdkGas)).CacheContext() // make sure we charge the higher level context even on panic