[HotFix] fix to charge gas cost for the key length in wasm execution

CHANGELOG.md

@@ -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.

x/wasm/keeper/contract.go

@@ -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(
@@ -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,
@@ -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
 }

x/wasm/keeper/submsg_test.go

@@ -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,
@@ -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,

x/wasm/types/gas_store.go

@@ -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)
+}

x/wasm/types/gas_store_test.go

@@ -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")
+}