diff --git a/go.mod b/go.mod index 83d772a744c..7abfc5711c0 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/CosmWasm/wasmd go 1.13 require ( - github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2 + github.com/CosmWasm/go-cosmwasm v0.10.0-alpha4 github.com/cosmos/cosmos-sdk v0.39.1-0.20200727135228-9d00f712e334 github.com/golang/mock v1.4.3 // indirect github.com/google/gofuzz v1.0.0 diff --git a/go.sum b/go.sum index 1d8cc7b1b4d..3d7073067e0 100644 --- a/go.sum +++ b/go.sum @@ -9,8 +9,8 @@ github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d h1:nalkkPQcITbvhmL4+C4cKA87NW0tfm3Kl9VXRoPywFg= github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d/go.mod h1:URdX5+vg25ts3aCh8H5IFZybJYKWhJHYMTnf+ULtoC4= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2 h1:k069wQF0f24w3A52mjR3AK6AfkuvQ5d0Mdu/rpB5nEk= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha4 h1:1a3j/vdhnyYvUV+67Hg3GU87M9wn1jR6bReXDwy+TZQ= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha4/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= diff --git a/x/wasm/internal/keeper/query_plugins.go b/x/wasm/internal/keeper/query_plugins.go index e20e872b921..bc76ca15400 100644 --- a/x/wasm/internal/keeper/query_plugins.go +++ b/x/wasm/internal/keeper/query_plugins.go @@ -2,7 +2,6 @@ package keeper import ( "encoding/json" - wasmTypes "github.com/CosmWasm/go-cosmwasm/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -17,18 +16,28 @@ type QueryHandler struct { var _ wasmTypes.Querier = QueryHandler{} -func (q QueryHandler) Query(request wasmTypes.QueryRequest) ([]byte, error) { +func (q QueryHandler) Query(request wasmTypes.QueryRequest, gasLimit uint64) ([]byte, error) { + // set a limit for a subctx + sdkGas := gasLimit / GasMultiplier + subctx := q.Ctx.WithGasMeter(sdk.NewGasMeter(sdkGas)) + + // make sure we charge the higher level context even on panic + defer func() { + q.Ctx.GasMeter().ConsumeGas(subctx.GasMeter().GasConsumed(), "contract sub-query") + }() + + // do the query if request.Bank != nil { - return q.Plugins.Bank(q.Ctx, request.Bank) + return q.Plugins.Bank(subctx, request.Bank) } if request.Custom != nil { - return q.Plugins.Custom(q.Ctx, request.Custom) + return q.Plugins.Custom(subctx, request.Custom) } if request.Staking != nil { - return q.Plugins.Staking(q.Ctx, request.Staking) + return q.Plugins.Staking(subctx, request.Staking) } if request.Wasm != nil { - return q.Plugins.Wasm(q.Ctx, request.Wasm) + return q.Plugins.Wasm(subctx, request.Wasm) } return nil, wasmTypes.Unknown{} } diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index fc9f55dcd6e..6d8884bc7a3 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -2,15 +2,16 @@ package keeper import ( "encoding/json" - abci "github.com/tendermint/tendermint/abci/types" "io/ioutil" "os" "testing" + wasmTypes "github.com/CosmWasm/go-cosmwasm/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" + abci "github.com/tendermint/tendermint/abci/types" ) type Recurse struct { @@ -34,6 +35,50 @@ type recurseResponse struct { Hashed []byte `json:"hashed"` } +// number os wasm queries called from a contract +var totalWasmQueryCounter int + +func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.AccAddress, ctx sdk.Context, keeper Keeper, cleanup func()) { + // we do one basic setup before all test cases (which are read-only and don't change state) + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + cleanup = func() { os.RemoveAll(tempDir) } + + var realWasmQuerier func(ctx sdk.Context, request *wasmTypes.WasmQuery) ([]byte, error) + countingQuerier := &QueryPlugins{ + Wasm: func(ctx sdk.Context, request *wasmTypes.WasmQuery) ([]byte, error) { + totalWasmQueryCounter++ + return realWasmQuerier(ctx, request) + }, + } + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, countingQuerier) + accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + realWasmQuerier = WasmQuerier(&keeper) + + deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) + creator = createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) + + // store the code + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + codeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) + require.NoError(t, err) + + // instantiate the contract + _, _, bob := keyPubAddr() + _, _, fred := keyPubAddr() + initMsg := InitMsg{ + Verifier: fred, + Beneficiary: bob, + } + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + contractAddr, err := keeper.Instantiate(ctx, codeID, creator, nil, initMsgBz, "recursive contract", deposit) + require.NoError(t, err) + + return contractAddr, creator, ctx, keeper, cleanup +} + func TestGasCostOnQuery(t *testing.T) { const ( GasNoWork uint64 = InstanceCost + 2_756 @@ -87,33 +132,8 @@ func TestGasCostOnQuery(t *testing.T) { }, } - // we do one basic setup before all test cases (which are read-only and don't change state) - tempDir, err := ioutil.TempDir("", "wasm") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) - accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper - deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) - creator := createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) - - // store the code - wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") - require.NoError(t, err) - codeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) - require.NoError(t, err) - - // instantiate the contract - _, _, bob := keyPubAddr() - _, _, fred := keyPubAddr() - initMsg := InitMsg{ - Verifier: fred, - Beneficiary: bob, - } - initMsgBz, err := json.Marshal(initMsg) - require.NoError(t, err) - contractAddr, err := keeper.Instantiate(ctx, codeID, creator, nil, initMsgBz, "recursive contract", deposit) - require.NoError(t, err) + contractAddr, creator, ctx, keeper, cleanup := initRecurseContract(t) + defer cleanup() for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -149,8 +169,7 @@ func TestGasCostOnQuery(t *testing.T) { func TestGasOnExternalQuery(t *testing.T) { const ( - GasWork50 uint64 = InstanceCost + 8_464 - GasReturnHashed uint64 = 597 + GasWork50 uint64 = InstanceCost + 8_464 ) cases := map[string]struct { @@ -190,33 +209,8 @@ func TestGasOnExternalQuery(t *testing.T) { }, } - // we do one basic setup before all test cases (which are read-only and don't change state) - tempDir, err := ioutil.TempDir("", "wasm") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) - accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper - deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) - creator := createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) - - // store the code - wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") - require.NoError(t, err) - codeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) - require.NoError(t, err) - - // instantiate the contract - _, _, bob := keyPubAddr() - _, _, fred := keyPubAddr() - initMsg := InitMsg{ - Verifier: fred, - Beneficiary: bob, - } - initMsgBz, err := json.Marshal(initMsg) - require.NoError(t, err) - contractAddr, err := keeper.Instantiate(ctx, codeID, creator, nil, initMsgBz, "recursive contract", deposit) - require.NoError(t, err) + contractAddr, _, ctx, keeper, cleanup := initRecurseContract(t) + defer cleanup() for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -233,7 +227,8 @@ func TestGasOnExternalQuery(t *testing.T) { if tc.expectPanic { require.Panics(t, func() { // this should run out of gas - _, _ = NewQuerier(keeper)(ctx, path, req) + _, err := NewQuerier(keeper)(ctx, path, req) + t.Logf("%v", err) }) } else { // otherwise, make sure we get a good success @@ -243,3 +238,95 @@ func TestGasOnExternalQuery(t *testing.T) { }) } } + +func TestLimitRecursiveQueryGas(t *testing.T) { + // The point of this test from https://github.com/CosmWasm/cosmwasm/issues/456 + // Basically, if I burn 90% of gas in CPU loop, then query out (to my self) + // the sub-query will have all the original gas (minus the 40k instance charge) + // and can burn 90% and call a sub-contract again... + // This attack would allow us to use far more than the provided gas before + // eventually hitting an OutOfGas panic. + + const ( + // Note: about 100 SDK gas (10k wasmer gas) for each round of sha256 + GasWork2k uint64 = InstanceCost + 233_379 // we have 6x gas used in cpu than in the instance + // This is overhead for calling into a sub-contract + GasReturnHashed uint64 = 603 + ) + + cases := map[string]struct { + gasLimit uint64 + msg Recurse + expectQueriesFromContract int + expectedGas uint64 + expectOutOfGas bool + }{ + "no recursion, lots of work": { + gasLimit: 4_000_000, + msg: Recurse{ + Depth: 0, + Work: 2000, + }, + expectQueriesFromContract: 0, + expectedGas: GasWork2k, + }, + "recursion 5, lots of work": { + gasLimit: 4_000_000, + msg: Recurse{ + Depth: 5, + Work: 2000, + }, + expectQueriesFromContract: 5, + expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed), + }, + // this is where we expect an error... + // it has enough gas to run 4 times and die on the 5th (4th time dispatching to sub-contract) + // however, if we don't charge the cpu gas before sub-dispatching, we can recurse over 20 times + // TODO: figure out how to asset how deep it went + "deep recursion, should die on 5th level": { + gasLimit: 1_200_000, + msg: Recurse{ + Depth: 50, + Work: 2000, + }, + expectQueriesFromContract: 4, + expectOutOfGas: true, + }, + } + + contractAddr, _, ctx, keeper, cleanup := initRecurseContract(t) + defer cleanup() + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + // reset the counter before test + totalWasmQueryCounter = 0 + + // make sure we set a limit before calling + ctx = ctx.WithGasMeter(sdk.NewGasMeter(tc.gasLimit)) + require.Equal(t, uint64(0), ctx.GasMeter().GasConsumed()) + + // prepare the query + recurse := tc.msg + recurse.Contract = contractAddr + msg := buildQuery(t, recurse) + + // if we expect out of gas, make sure this panics + if tc.expectOutOfGas { + require.Panics(t, func() { + _, err := keeper.QuerySmart(ctx, contractAddr, msg) + t.Logf("Got error not panic: %#v", err) + }) + assert.Equal(t, tc.expectQueriesFromContract, totalWasmQueryCounter) + return + } + + // otherwise, we expect a successful call + _, err := keeper.QuerySmart(ctx, contractAddr, msg) + require.NoError(t, err) + assert.Equal(t, tc.expectedGas, ctx.GasMeter().GasConsumed()) + + assert.Equal(t, tc.expectQueriesFromContract, totalWasmQueryCounter) + }) + } +}