Skip to content

Commit

Permalink
[HotFix] fix to charge gas cost for the key length in wasm execution (#…
Browse files Browse the repository at this point in the history
…674)

* use custom gas store to charge gas cost for the key length

* add changelog
  • Loading branch information
yun-yeo authored Feb 6, 2022
1 parent 2b04c05 commit 293b023
Show file tree
Hide file tree
Showing 5 changed files with 338 additions and 6 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
## v0.5.16

This release contains security update

- [\#674](https://github.com/terra-money/core/pull/674) fix to charge gas cost for the key length in wasm execution

### Softfork Schedule

| | columbus-5 | bombay-12 |
| ------------- | ------------------------- | ------------------------- |
| Height | 6,470,000 | 7,800,000 |
| Expected Time | 2022-02-13T15:00:00 (UTC) | 2022-02-11T15:00:00 (UTC) |

## v0.5.15

This release contains mainly overflow checking enhancement including custom dependency updates.
Expand Down
6 changes: 3 additions & 3 deletions x/wasm/keeper/contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (k Keeper) InstantiateContract(

// create prefixed data store
contractStoreKey := types.GetContractStoreKey(contractAddress)
contractStore := prefix.NewStore(ctx.KVStore(k.storeKey), contractStoreKey)
contractStore := prefix.NewStore(types.KVStore(ctx, k.storeKey), contractStoreKey)

// instantiate wasm contract
res, gasUsed, err := k.wasmVM.Instantiate(
Expand Down Expand Up @@ -302,7 +302,7 @@ func (k Keeper) MigrateContract(

// prepare necessary meta data
prefixStoreKey := types.GetContractStoreKey(contractAddress)
prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), prefixStoreKey)
prefixStore := prefix.NewStore(types.KVStore(ctx, k.storeKey), prefixStoreKey)

res, gasUsed, err := k.wasmVM.Migrate(
newCodeInfo.CodeHash,
Expand Down Expand Up @@ -492,6 +492,6 @@ func (k Keeper) getContractDetails(ctx sdk.Context, contractAddress sdk.AccAddre

k.cdc.MustUnmarshal(bz, &codeInfo)
contractStoreKey := types.GetContractStoreKey(contractAddress)
contractStorePrefix = prefix.NewStore(ctx.KVStore(k.storeKey), contractStoreKey)
contractStorePrefix = prefix.NewStore(types.KVStore(ctx, k.storeKey), contractStoreKey)
return
}
6 changes: 3 additions & 3 deletions x/wasm/keeper/submsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
msg: invalidBankSend,
subMsgError: true,
// uses less gas than the send tokens (cost of bank transfer)
resultAssertions: []assertion{assertGasUsed(98000, 100000), assertErrorString("insufficient funds")},
resultAssertions: []assertion{assertGasUsed(100000, 101000), assertErrorString("insufficient funds")},
},
"out of gas panic with no gas limit": {
submsgID: 7,
Expand All @@ -302,15 +302,15 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
subMsgError: true,
gasLimit: &subGasLimit,
// uses same gas as call without limit
resultAssertions: []assertion{assertGasUsed(98000, 100000), assertErrorString("insufficient funds")},
resultAssertions: []assertion{assertGasUsed(100000, 101000), assertErrorString("insufficient funds")},
},
"out of gas caught with gas limit": {
submsgID: 17,
msg: infiniteLoop,
subMsgError: true,
gasLimit: &subGasLimit,
// uses all the subGasLimit, plus the 92k or so for the main contract
resultAssertions: []assertion{assertGasUsed(subGasLimit+92000, subGasLimit+94000), assertErrorString("out of gas")},
resultAssertions: []assertion{assertGasUsed(subGasLimit+93000, subGasLimit+95000), assertErrorString("out of gas")},
},
"instantiate contract gets address in data and events": {
submsgID: 21,
Expand Down
199 changes: 199 additions & 0 deletions x/wasm/types/gas_store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
package types

import (
"io"
"time"

"github.com/cosmos/cosmos-sdk/store/gaskv"
"github.com/cosmos/cosmos-sdk/store/types"
stypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
)

var _ types.KVStore = &Store{}

// KVStore return new gas KVStore which fixed
// https://github.com/cosmos/cosmos-sdk/issues/10243
func KVStore(ctx sdk.Context, key sdk.StoreKey) types.KVStore {
if (ctx.ChainID() == "bombay-12" && ctx.BlockHeight() < 7_800_000) ||
(ctx.ChainID() == "columbus-5" && ctx.BlockHeight() < 6_470_000) {
return gaskv.NewStore(ctx.MultiStore().GetKVStore(key), ctx.GasMeter(), stypes.KVGasConfig())
}

return NewStore(ctx.MultiStore().GetKVStore(key), ctx.GasMeter(), stypes.KVGasConfig())
}

// Store applies gas tracking to an underlying KVStore. It implements the
// KVStore interface.
type Store struct {
gasMeter types.GasMeter
gasConfig types.GasConfig
parent types.KVStore
}

// NewStore returns a reference to a new GasKVStore.
func NewStore(parent types.KVStore, gasMeter types.GasMeter, gasConfig types.GasConfig) *Store {
kvs := &Store{
gasMeter: gasMeter,
gasConfig: gasConfig,
parent: parent,
}
return kvs
}

// GetStoreType implements Store.
func (gs *Store) GetStoreType() types.StoreType {
return gs.parent.GetStoreType()
}

// Get implements KVStore.
func (gs *Store) Get(key []byte) (value []byte) {
gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostFlat, types.GasReadCostFlatDesc)
value = gs.parent.Get(key)

// TODO overflow-safe math?
gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostPerByte*types.Gas(len(key)), types.GasReadPerByteDesc)
gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasReadPerByteDesc)

return value
}

// Set implements KVStore.
func (gs *Store) Set(key []byte, value []byte) {
types.AssertValidKey(key)
types.AssertValidValue(value)
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostFlat, types.GasWriteCostFlatDesc)
// TODO overflow-safe math?
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostPerByte*types.Gas(len(key)), types.GasWritePerByteDesc)
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostPerByte*types.Gas(len(value)), types.GasWritePerByteDesc)
gs.parent.Set(key, value)
}

// Has implements KVStore.
func (gs *Store) Has(key []byte) bool {
defer telemetry.MeasureSince(time.Now(), "store", "gaskv", "has")
gs.gasMeter.ConsumeGas(gs.gasConfig.HasCost, types.GasHasDesc)
return gs.parent.Has(key)
}

// Delete implements KVStore.
func (gs *Store) Delete(key []byte) {
defer telemetry.MeasureSince(time.Now(), "store", "gaskv", "delete")
// charge gas to prevent certain attack vectors even though space is being freed
gs.gasMeter.ConsumeGas(gs.gasConfig.DeleteCost, types.GasDeleteDesc)
gs.parent.Delete(key)
}

// Iterator implements the KVStore interface. It returns an iterator which
// incurs a flat gas cost for seeking to the first key/value pair and a variable
// gas cost based on the current value's length if the iterator is valid.
func (gs *Store) Iterator(start, end []byte) types.Iterator {
return gs.iterator(start, end, true)
}

// ReverseIterator implements the KVStore interface. It returns a reverse
// iterator which incurs a flat gas cost for seeking to the first key/value pair
// and a variable gas cost based on the current value's length if the iterator
// is valid.
func (gs *Store) ReverseIterator(start, end []byte) types.Iterator {
return gs.iterator(start, end, false)
}

// CacheWrap implements KVStore.
func (gs *Store) CacheWrap() types.CacheWrap {
panic("cannot CacheWrap a GasKVStore")
}

// CacheWrapWithTrace implements the KVStore interface.
func (gs *Store) CacheWrapWithTrace(_ io.Writer, _ types.TraceContext) types.CacheWrap {
panic("cannot CacheWrapWithTrace a GasKVStore")
}

// CacheWrapWithListeners implements the CacheWrapper interface.
func (gs *Store) CacheWrapWithListeners(_ types.StoreKey, _ []types.WriteListener) types.CacheWrap {
panic("cannot CacheWrapWithListeners a GasKVStore")
}

func (gs *Store) iterator(start, end []byte, ascending bool) types.Iterator {
var parent types.Iterator
if ascending {
parent = gs.parent.Iterator(start, end)
} else {
parent = gs.parent.ReverseIterator(start, end)
}

gi := newGasIterator(gs.gasMeter, gs.gasConfig, parent)
gi.(*gasIterator).consumeSeekGas()

return gi
}

type gasIterator struct {
gasMeter types.GasMeter
gasConfig types.GasConfig
parent types.Iterator
}

func newGasIterator(gasMeter types.GasMeter, gasConfig types.GasConfig, parent types.Iterator) types.Iterator {
return &gasIterator{
gasMeter: gasMeter,
gasConfig: gasConfig,
parent: parent,
}
}

// Implements Iterator.
func (gi *gasIterator) Domain() (start []byte, end []byte) {
return gi.parent.Domain()
}

// Implements Iterator.
func (gi *gasIterator) Valid() bool {
return gi.parent.Valid()
}

// Next implements the Iterator interface. It seeks to the next key/value pair
// in the iterator. It incurs a flat gas cost for seeking and a variable gas
// cost based on the current value's length if the iterator is valid.
func (gi *gasIterator) Next() {
gi.consumeSeekGas()
gi.parent.Next()
}

// Key implements the Iterator interface. It returns the current key and it does
// not incur any gas cost.
func (gi *gasIterator) Key() (key []byte) {
key = gi.parent.Key()
return key
}

// Value implements the Iterator interface. It returns the current value and it
// does not incur any gas cost.
func (gi *gasIterator) Value() (value []byte) {
value = gi.parent.Value()
return value
}

// Implements Iterator.
func (gi *gasIterator) Close() error {
return gi.parent.Close()
}

// Error delegates the Error call to the parent iterator.
func (gi *gasIterator) Error() error {
return gi.parent.Error()
}

// consumeSeekGas consumes on each iteration step a flat gas cost and a variable gas cost
// based on the current value's length.
func (gi *gasIterator) consumeSeekGas() {
if gi.Valid() {
key := gi.Key()
value := gi.Value()

gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*types.Gas(len(key)), types.GasValuePerByteDesc)
gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasValuePerByteDesc)
}
gi.gasMeter.ConsumeGas(gi.gasConfig.IterNextCostFlat, types.GasIterNextCostFlatDesc)
}
120 changes: 120 additions & 0 deletions x/wasm/types/gas_store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package types

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
dbm "github.com/tendermint/tm-db"

"github.com/cosmos/cosmos-sdk/store/dbadapter"
"github.com/cosmos/cosmos-sdk/store/types"
)

func bz(s string) []byte { return []byte(s) }

func keyFmt(i int) []byte { return bz(fmt.Sprintf("key%0.8d", i)) }
func valFmt(i int) []byte { return bz(fmt.Sprintf("value%0.8d", i)) }

func TestGasKVStoreBasic(t *testing.T) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
meter := types.NewGasMeter(10000)
st := NewStore(mem, meter, types.KVGasConfig())

require.Equal(t, types.StoreTypeDB, st.GetStoreType())
require.Panics(t, func() { st.CacheWrap() })
require.Panics(t, func() { st.CacheWrapWithTrace(nil, nil) })
require.Panics(t, func() { st.CacheWrapWithListeners(nil, nil) })

require.Panics(t, func() { st.Set(nil, []byte("value")) }, "setting a nil key should panic")
require.Panics(t, func() { st.Set([]byte(""), []byte("value")) }, "setting an empty key should panic")

require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
st.Set(keyFmt(1), valFmt(1))
require.Equal(t, valFmt(1), st.Get(keyFmt(1)))
st.Delete(keyFmt(1))
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Equal(t, meter.GasConsumed(), types.Gas(6858))
}

func TestGasKVStoreIterator(t *testing.T) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
meter := types.NewGasMeter(100000)
st := NewStore(mem, meter, types.KVGasConfig())
require.False(t, st.Has(keyFmt(1)))
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Empty(t, st.Get(keyFmt(2)), "Expected `key2` to be empty")
require.Empty(t, st.Get(keyFmt(3)), "Expected `key3` to be empty")

st.Set(keyFmt(1), valFmt(1))
require.True(t, st.Has(keyFmt(1)))
st.Set(keyFmt(2), valFmt(2))
require.True(t, st.Has(keyFmt(2)))
st.Set(keyFmt(3), valFmt(0))

iterator := st.Iterator(nil, nil)
start, end := iterator.Domain()
require.Nil(t, start)
require.Nil(t, end)
require.NoError(t, iterator.Error())

t.Cleanup(func() {
if err := iterator.Close(); err != nil {
t.Fatal(err)
}
})
ka := iterator.Key()
require.Equal(t, ka, keyFmt(1))
va := iterator.Value()
require.Equal(t, va, valFmt(1))
iterator.Next()
kb := iterator.Key()
require.Equal(t, kb, keyFmt(2))
vb := iterator.Value()
require.Equal(t, vb, valFmt(2))
iterator.Next()
require.Equal(t, types.Gas(14565), meter.GasConsumed())
kc := iterator.Key()
require.Equal(t, kc, keyFmt(3))
vc := iterator.Value()
require.Equal(t, vc, valFmt(0))
iterator.Next()
require.Equal(t, types.Gas(14667), meter.GasConsumed())
require.False(t, iterator.Valid())
require.Panics(t, iterator.Next)
require.Equal(t, types.Gas(14697), meter.GasConsumed())
require.NoError(t, iterator.Error())

reverseIterator := st.ReverseIterator(nil, nil)
t.Cleanup(func() {
if err := reverseIterator.Close(); err != nil {
t.Fatal(err)
}
})
require.Equal(t, reverseIterator.Key(), keyFmt(3))
reverseIterator.Next()
require.Equal(t, reverseIterator.Key(), keyFmt(2))
reverseIterator.Next()
require.Equal(t, reverseIterator.Key(), keyFmt(1))
reverseIterator.Next()
require.False(t, reverseIterator.Valid())
require.Panics(t, reverseIterator.Next)
require.Equal(t, types.Gas(15135), meter.GasConsumed())
}

func TestGasKVStoreOutOfGasSet(t *testing.T) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
meter := types.NewGasMeter(0)
st := NewStore(mem, meter, types.KVGasConfig())
require.Panics(t, func() { st.Set(keyFmt(1), valFmt(1)) }, "Expected out-of-gas")
}

func TestGasKVStoreOutOfGasIterator(t *testing.T) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
meter := types.NewGasMeter(20000)
st := NewStore(mem, meter, types.KVGasConfig())
st.Set(keyFmt(1), valFmt(1))
iterator := st.Iterator(nil, nil)
iterator.Next()
require.Panics(t, func() { iterator.Value() }, "Expected out-of-gas")
}

0 comments on commit 293b023

Please sign in to comment.