Skip to content

Commit

Permalink
prevent overflow by blocking huge input at handler & querier
Browse files Browse the repository at this point in the history
  • Loading branch information
Yun committed Nov 24, 2019
1 parent 6bc6f38 commit c11c905
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 25 deletions.
29 changes: 14 additions & 15 deletions x/market/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ import (

const (
DefaultCodespace = types.DefaultCodespace
CodeInsufficientSwap = types.CodeInsufficientSwap
CodeInsufficientSwap = types.CodeInvalidOfferCoin
CodeNoEffectivePrice = types.CodeNoEffectivePrice
CodeRecursiveSwap = types.CodeRecursiveSwap
CodeInactive = types.CodeInactive
ModuleName = types.ModuleName
StoreKey = types.StoreKey
RouterKey = types.RouterKey
Expand All @@ -28,19 +27,19 @@ const (

var (
// functions aliases
RegisterCodec = types.RegisterCodec
ErrNoEffectivePrice = types.ErrNoEffectivePrice
ErrInsufficientSwapCoins = types.ErrInsufficientSwapCoins
ErrRecursiveSwap = types.ErrRecursiveSwap
NewGenesisState = types.NewGenesisState
DefaultGenesisState = types.DefaultGenesisState
ValidateGenesis = types.ValidateGenesis
NewMsgSwap = types.NewMsgSwap
DefaultParams = types.DefaultParams
NewQuerySwapParams = types.NewQuerySwapParams
NewKeeper = keeper.NewKeeper
ParamKeyTable = keeper.ParamKeyTable
NewQuerier = keeper.NewQuerier
RegisterCodec = types.RegisterCodec
ErrNoEffectivePrice = types.ErrNoEffectivePrice
ErrInvalidOfferCoin = types.ErrInvalidOfferCoin
ErrRecursiveSwap = types.ErrRecursiveSwap
NewGenesisState = types.NewGenesisState
DefaultGenesisState = types.DefaultGenesisState
ValidateGenesis = types.ValidateGenesis
NewMsgSwap = types.NewMsgSwap
DefaultParams = types.DefaultParams
NewQuerySwapParams = types.NewQuerySwapParams
NewKeeper = keeper.NewKeeper
ParamKeyTable = keeper.ParamKeyTable
NewQuerier = keeper.NewQuerier

// variable aliases
ModuleCdc = types.ModuleCdc
Expand Down
4 changes: 4 additions & 0 deletions x/market/internal/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func querySwap(ctx sdk.Context, req abci.RequestQuery, keeper Keeper) ([]byte, s
return nil, types.ErrRecursiveSwap(types.DefaultCodespace, params.AskDenom)
}

if params.OfferCoin.Amount.BigInt().BitLen() > 100 {
return nil, types.ErrInvalidOfferCoin(keeper.Codespace(), params.OfferCoin.Amount)
}

swapCoin, spread, err := keeper.ComputeSwap(ctx, params.OfferCoin, params.AskDenom)
if err != nil {
return nil, sdk.ErrInternal(sdk.AppendMsgToErr("Failed to get swapped coin amount", err.Error()))
Expand Down
15 changes: 15 additions & 0 deletions x/market/internal/keeper/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ func TestQuerySwap(t *testing.T) {
res, err := querier(input.Ctx, []string{types.QuerySwap}, query)
require.Error(t, err)

// overflow query
overflowAmt, _ := sdk.NewIntFromString("1000000000000000000000000000000000")
overflowOfferCoin := sdk.NewCoin(core.MicroLunaDenom, overflowAmt)
queryParams = types.NewQuerySwapParams(overflowOfferCoin, core.MicroSDRDenom)
bz, err = cdc.MarshalJSON(queryParams)
require.NoError(t, err)

query = abci.RequestQuery{
Path: "",
Data: bz,
}

_, err = querier(input.Ctx, []string{types.QuerySwap}, query)
require.Error(t, err)

// valid query
queryParams = types.NewQuerySwapParams(offerCoin, core.MicroSDRDenom)
bz, err = cdc.MarshalJSON(queryParams)
Expand Down
2 changes: 1 addition & 1 deletion x/market/internal/keeper/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (k Keeper) ComputeInternalSwap(ctx sdk.Context, offerCoin sdk.DecCoin, askD

retAmount := offerCoin.Amount.Mul(askRate).Quo(offerRate)
if retAmount.LTE(sdk.ZeroDec()) {
return sdk.DecCoin{}, types.ErrInsufficientSwapCoins(types.DefaultCodespace, offerCoin.Amount.TruncateInt())
return sdk.DecCoin{}, types.ErrInvalidOfferCoin(types.DefaultCodespace, offerCoin.Amount.TruncateInt())
}

return sdk.NewDecCoinFromDec(askDenom, retAmount), nil
Expand Down
9 changes: 4 additions & 5 deletions x/market/internal/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ type codeType = sdk.CodeType
const (
DefaultCodespace sdk.CodespaceType = "market"

CodeInsufficientSwap codeType = 1
CodeInvalidOfferCoin codeType = 1
CodeNoEffectivePrice codeType = 2
CodeRecursiveSwap codeType = 3
CodeInactive codeType = 4
)

// ----------------------------------------
Expand All @@ -24,9 +23,9 @@ func ErrNoEffectivePrice(codespace sdk.CodespaceType, denom string) sdk.Error {
return sdk.NewError(codespace, CodeNoEffectivePrice, "No price registered with the oracle for asset: "+denom)
}

// ErrInsufficientSwapCoins called when not enough coins are being requested for a swap
func ErrInsufficientSwapCoins(codespace sdk.CodespaceType, rval sdk.Int) sdk.Error {
return sdk.NewError(codespace, CodeInsufficientSwap, "Not enough coins for a swap: "+rval.String())
// ErrInvalidOfferCoin called when not enough or too huge coins are being requested for a swap
func ErrInvalidOfferCoin(codespace sdk.CodespaceType, rval sdk.Int) sdk.Error {
return sdk.NewError(codespace, CodeInvalidOfferCoin, "Invalid offer coin for a swap: "+rval.String())
}

// ErrRecursiveSwap called when Ask and Offer coin denominatioins are equal
Expand Down
4 changes: 2 additions & 2 deletions x/market/internal/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ func (msg MsgSwap) ValidateBasic() sdk.Error {
return sdk.ErrInvalidAddress("Invalid address: " + msg.Trader.String())
}

if msg.OfferCoin.Amount.LTE(sdk.ZeroInt()) {
return ErrInsufficientSwapCoins(DefaultCodespace, msg.OfferCoin.Amount)
if msg.OfferCoin.Amount.LTE(sdk.ZeroInt()) || msg.OfferCoin.Amount.BigInt().BitLen() > 100 {
return ErrInvalidOfferCoin(DefaultCodespace, msg.OfferCoin.Amount)
}

if msg.OfferCoin.Denom == msg.AskDenom {
Expand Down
3 changes: 3 additions & 0 deletions x/market/internal/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
func TestMsgSwap(t *testing.T) {
_, addrs, _, _ := mock.CreateGenAccounts(1, sdk.Coins{})

overflowOfferAmt, _ := sdk.NewIntFromString("100000000000000000000000000000000000000000000000000000000")

tests := []struct {
trader sdk.AccAddress
offerCoin sdk.Coin
Expand All @@ -22,6 +24,7 @@ func TestMsgSwap(t *testing.T) {
{addrs[0], sdk.NewCoin(core.MicroLunaDenom, sdk.OneInt()), core.MicroSDRDenom, true},
{sdk.AccAddress{}, sdk.NewCoin(core.MicroLunaDenom, sdk.OneInt()), core.MicroSDRDenom, false},
{addrs[0], sdk.NewCoin(core.MicroLunaDenom, sdk.ZeroInt()), core.MicroSDRDenom, false},
{addrs[0], sdk.NewCoin(core.MicroLunaDenom, overflowOfferAmt), core.MicroSDRDenom, false},
{addrs[0], sdk.NewCoin(core.MicroLunaDenom, sdk.OneInt()), core.MicroLunaDenom, false},
}

Expand Down
3 changes: 2 additions & 1 deletion x/oracle/internal/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ func (msg MsgExchangeRateVote) ValidateBasic() sdk.Error {
return sdk.ErrInvalidAddress("Invalid address: " + msg.Feeder.String())
}

if msg.ExchangeRate.LTE(sdk.ZeroDec()) {
// Check overflow bit length
if msg.ExchangeRate.BitLen() > 100+sdk.DecimalPrecisionBits {
return ErrInvalidExchangeRate(DefaultCodespace, msg.ExchangeRate)
}

Expand Down
4 changes: 3 additions & 1 deletion x/oracle/internal/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func TestMsgExchangeRatePrevote(t *testing.T) {
func TestMsgExchangeRateVote(t *testing.T) {
_, addrs, _, _ := mock.CreateGenAccounts(1, sdk.Coins{})

overflowExchangeRate, _ := sdk.NewDecFromStr("100000000000000000000000000000000000000000000000000000000")

tests := []struct {
denom string
voter sdk.AccAddress
Expand All @@ -52,7 +54,7 @@ func TestMsgExchangeRateVote(t *testing.T) {
}{
{"", addrs[0], "123", sdk.OneDec(), false},
{core.MicroCNYDenom, addrs[0], "123", sdk.OneDec().MulInt64(core.MicroUnit), true},
{core.MicroCNYDenom, addrs[0], "123", sdk.ZeroDec(), false},
{core.MicroCNYDenom, addrs[0], "123", overflowExchangeRate, false},
{core.MicroCNYDenom, sdk.AccAddress{}, "123", sdk.OneDec().MulInt64(core.MicroUnit), false},
{core.MicroCNYDenom, addrs[0], "", sdk.OneDec().MulInt64(core.MicroUnit), false},
}
Expand Down

0 comments on commit c11c905

Please sign in to comment.