From a447ee0c121d3bf4a10ba0a49b0dd98c4c5449e3 Mon Sep 17 00:00:00 2001 From: Alexander Peters Date: Tue, 14 Sep 2021 17:25:05 +0200 Subject: [PATCH] Ensure query isolation (#611) * Ensure query isolation * Review feedback --- x/wasm/keeper/keeper_test.go | 31 +++++++++++++++++++++++++++++++ x/wasm/keeper/query_plugins.go | 9 +++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/x/wasm/keeper/keeper_test.go b/x/wasm/keeper/keeper_test.go index 4a43059b..835d10aa 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 b767cd99..2ac014d6 100644 --- a/x/wasm/keeper/query_plugins.go +++ b/x/wasm/keeper/query_plugins.go @@ -46,15 +46,16 @@ 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)) + // 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 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 {