Skip to content

Commit

Permalink
Merge pull request #233 from CosmWasm/restrict-querier-redispatched-gas
Browse files Browse the repository at this point in the history 
Restrict querier redispatched gas
  • Loading branch information
@ethanfrey
ethanfrey committed Jul 29, 2020
2 parents 789ef54 + d600958 commit 6c1f75b
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 67 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
21 changes: 15 additions & 6 deletions x/wasm/internal/keeper/query_plugins.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{}
}
Expand Down
203 changes: 145 additions & 58 deletions x/wasm/internal/keeper/recurse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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)
})
}
}

0 comments on commit 6c1f75b

Please sign in to comment.