Skip to content

Commit

Permalink
Merge PR cosmos#7053: Add address length validation to MsgSend and Ms…
Browse files Browse the repository at this point in the history 
…gMultiSend
  • Loading branch information
@webmaster128
webmaster128 authored Aug 17, 2020
1 parent 7d2062b commit 574c7d2
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 66 deletions.
18 changes: 9 additions & 9 deletions x/bank/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,15 @@ func (suite *IntegrationTestSuite) TestInputOutputNewAccount() {
app, ctx := suite.app, suite.ctx

balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50))
addr1 := sdk.AccAddress([]byte("addr1"))
addr1 := sdk.AccAddress([]byte("addr1_______________"))
acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
app.AccountKeeper.SetAccount(ctx, acc1)
suite.Require().NoError(app.BankKeeper.SetBalances(ctx, addr1, balances))

acc1Balances := app.BankKeeper.GetAllBalances(ctx, addr1)
suite.Require().Equal(balances, acc1Balances)

addr2 := sdk.AccAddress([]byte("addr2"))
addr2 := sdk.AccAddress([]byte("addr2_______________"))

suite.Require().Nil(app.AccountKeeper.GetAccount(ctx, addr2))
suite.Require().Empty(app.BankKeeper.GetAllBalances(ctx, addr2))
Expand All @@ -329,15 +329,15 @@ func (suite *IntegrationTestSuite) TestInputOutputCoins() {
app, ctx := suite.app, suite.ctx
balances := sdk.NewCoins(newFooCoin(90), newBarCoin(30))

addr1 := sdk.AccAddress([]byte("addr1"))
addr1 := sdk.AccAddress([]byte("addr1_______________"))
acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
app.AccountKeeper.SetAccount(ctx, acc1)

addr2 := sdk.AccAddress([]byte("addr2"))
addr2 := sdk.AccAddress([]byte("addr2_______________"))
acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2)
app.AccountKeeper.SetAccount(ctx, acc2)

addr3 := sdk.AccAddress([]byte("addr3"))
addr3 := sdk.AccAddress([]byte("addr3_______________"))
acc3 := app.AccountKeeper.NewAccountWithAddress(ctx, addr3)
app.AccountKeeper.SetAccount(ctx, acc3)

Expand Down Expand Up @@ -576,10 +576,10 @@ func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {

app.BankKeeper.SetParams(ctx, types.DefaultParams())

addr := sdk.AccAddress([]byte("addr1"))
addr2 := sdk.AccAddress([]byte("addr2"))
addr3 := sdk.AccAddress([]byte("addr3"))
addr4 := sdk.AccAddress([]byte("addr4"))
addr := sdk.AccAddress([]byte("addr1_______________"))
addr2 := sdk.AccAddress([]byte("addr2_______________"))
addr3 := sdk.AccAddress([]byte("addr3_______________"))
addr4 := sdk.AccAddress([]byte("addr4_______________"))
acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2)

Expand Down
16 changes: 8 additions & 8 deletions x/bank/types/msgs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ func (msg MsgSend) Type() string { return TypeMsgSend }

// ValidateBasic Implements Msg.
func (msg MsgSend) ValidateBasic() error {
if msg.FromAddress.Empty() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing sender address")
if err := sdk.VerifyAddressFormat(msg.FromAddress); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", err)
}

if msg.ToAddress.Empty() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing recipient address")
if err := sdk.VerifyAddressFormat(msg.ToAddress); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid recipient address (%s)", err)
}

if !msg.Amount.IsValid() {
Expand Down Expand Up @@ -100,8 +100,8 @@ func (msg MsgMultiSend) GetSigners() []sdk.AccAddress {

// ValidateBasic - validate transaction input
func (in Input) ValidateBasic() error {
if len(in.Address) == 0 {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "input address missing")
if err := sdk.VerifyAddressFormat(in.Address); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid input address (%s)", err)
}

if !in.Coins.IsValid() {
Expand All @@ -125,8 +125,8 @@ func NewInput(addr sdk.AccAddress, coins sdk.Coins) Input {

// ValidateBasic - validate transaction output
func (out Output) ValidateBasic() error {
if len(out.Address) == 0 {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "output address missing")
if err := sdk.VerifyAddressFormat(out.Address); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid output address (%s)", err)
}

if !out.Coins.IsValid() {
Expand Down
107 changes: 58 additions & 49 deletions x/bank/types/msgs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,33 +20,36 @@ func TestMsgSendRoute(t *testing.T) {
}

func TestMsgSendValidation(t *testing.T) {
addr1 := sdk.AccAddress([]byte("from"))
addr2 := sdk.AccAddress([]byte("to"))
addr1 := sdk.AccAddress([]byte("from________________"))
addr2 := sdk.AccAddress([]byte("to__________________"))
addrEmpty := sdk.AccAddress([]byte(""))
addrTooLong := sdk.AccAddress([]byte("Accidentally used 33 bytes pubkey"))

atom123 := sdk.NewCoins(sdk.NewInt64Coin("atom", 123))
atom0 := sdk.NewCoins(sdk.NewInt64Coin("atom", 0))
atom123eth123 := sdk.NewCoins(sdk.NewInt64Coin("atom", 123), sdk.NewInt64Coin("eth", 123))
atom123eth0 := sdk.Coins{sdk.NewInt64Coin("atom", 123), sdk.NewInt64Coin("eth", 0)}

var emptyAddr sdk.AccAddress

cases := []struct {
valid bool
tx *MsgSend
expectedErr string // empty means no error expected
msg *MsgSend
}{
{true, NewMsgSend(addr1, addr2, atom123)}, // valid send
{true, NewMsgSend(addr1, addr2, atom123eth123)}, // valid send with multiple coins
{false, NewMsgSend(addr1, addr2, atom0)}, // non positive coin
{false, NewMsgSend(addr1, addr2, atom123eth0)}, // non positive coin in multicoins
{false, NewMsgSend(emptyAddr, addr2, atom123)}, // empty from addr
{false, NewMsgSend(addr1, emptyAddr, atom123)}, // empty to addr
{"", NewMsgSend(addr1, addr2, atom123)}, // valid send
{"", NewMsgSend(addr1, addr2, atom123eth123)}, // valid send with multiple coins
{": invalid coins", NewMsgSend(addr1, addr2, atom0)}, // non positive coin
{"123atom,0eth: invalid coins", NewMsgSend(addr1, addr2, atom123eth0)}, // non positive coin in multicoins
{"Invalid sender address (incorrect address length (expected: 20, actual: 0)): invalid address", NewMsgSend(addrEmpty, addr2, atom123)},
{"Invalid sender address (incorrect address length (expected: 20, actual: 33)): invalid address", NewMsgSend(addrTooLong, addr2, atom123)},
{"Invalid recipient address (incorrect address length (expected: 20, actual: 0)): invalid address", NewMsgSend(addr1, addrEmpty, atom123)},
{"Invalid recipient address (incorrect address length (expected: 20, actual: 33)): invalid address", NewMsgSend(addr1, addrTooLong, atom123)},
}

for _, tc := range cases {
err := tc.tx.ValidateBasic()
if tc.valid {
err := tc.msg.ValidateBasic()
if tc.expectedErr == "" {
require.Nil(t, err)
} else {
require.NotNil(t, err)
require.EqualError(t, err, tc.expectedErr)
}
}
}
Expand Down Expand Up @@ -85,84 +88,90 @@ func TestMsgMultiSendRoute(t *testing.T) {
}

func TestInputValidation(t *testing.T) {
addr1 := sdk.AccAddress([]byte{1, 2})
addr2 := sdk.AccAddress([]byte{7, 8})
addr1 := sdk.AccAddress([]byte("_______alice________"))
addr2 := sdk.AccAddress([]byte("________bob_________"))
addrEmpty := sdk.AccAddress([]byte(""))
addrTooLong := sdk.AccAddress([]byte("Accidentally used 33 bytes pubkey"))

someCoins := sdk.NewCoins(sdk.NewInt64Coin("atom", 123))
multiCoins := sdk.NewCoins(sdk.NewInt64Coin("atom", 123), sdk.NewInt64Coin("eth", 20))

var emptyAddr sdk.AccAddress
emptyCoins := sdk.NewCoins()
emptyCoins2 := sdk.NewCoins(sdk.NewInt64Coin("eth", 0))
someEmptyCoins := sdk.Coins{sdk.NewInt64Coin("eth", 10), sdk.NewInt64Coin("atom", 0)}
unsortedCoins := sdk.Coins{sdk.NewInt64Coin("eth", 1), sdk.NewInt64Coin("atom", 1)}

cases := []struct {
valid bool
txIn Input
expectedErr string // empty means no error expected
txIn Input
}{
// auth works with different apps
{true, NewInput(addr1, someCoins)},
{true, NewInput(addr2, someCoins)},
{true, NewInput(addr2, multiCoins)},

{false, NewInput(emptyAddr, someCoins)}, // empty address
{false, NewInput(addr1, emptyCoins)}, // invalid coins
{false, NewInput(addr1, emptyCoins2)}, // invalid coins
{false, NewInput(addr1, someEmptyCoins)}, // invalid coins
{false, NewInput(addr1, unsortedCoins)}, // unsorted coins
{"", NewInput(addr1, someCoins)},
{"", NewInput(addr2, someCoins)},
{"", NewInput(addr2, multiCoins)},

{"Invalid input address (incorrect address length (expected: 20, actual: 0)): invalid address", NewInput(addrEmpty, someCoins)},
{"Invalid input address (incorrect address length (expected: 20, actual: 33)): invalid address", NewInput(addrTooLong, someCoins)},
{": invalid coins", NewInput(addr1, emptyCoins)}, // invalid coins
{": invalid coins", NewInput(addr1, emptyCoins2)}, // invalid coins
{"10eth,0atom: invalid coins", NewInput(addr1, someEmptyCoins)}, // invalid coins
{"1eth,1atom: invalid coins", NewInput(addr1, unsortedCoins)}, // unsorted coins
}

for i, tc := range cases {
err := tc.txIn.ValidateBasic()
if tc.valid {
if tc.expectedErr == "" {
require.Nil(t, err, "%d: %+v", i, err)
} else {
require.NotNil(t, err, "%d", i)
require.EqualError(t, err, tc.expectedErr, "%d", i)
}
}
}

func TestOutputValidation(t *testing.T) {
addr1 := sdk.AccAddress([]byte{1, 2})
addr2 := sdk.AccAddress([]byte{7, 8})
addr1 := sdk.AccAddress([]byte("_______alice________"))
addr2 := sdk.AccAddress([]byte("________bob_________"))
addrEmpty := sdk.AccAddress([]byte(""))
addrTooLong := sdk.AccAddress([]byte("Accidentally used 33 bytes pubkey"))

someCoins := sdk.NewCoins(sdk.NewInt64Coin("atom", 123))
multiCoins := sdk.NewCoins(sdk.NewInt64Coin("atom", 123), sdk.NewInt64Coin("eth", 20))

var emptyAddr sdk.AccAddress
emptyCoins := sdk.NewCoins()
emptyCoins2 := sdk.NewCoins(sdk.NewInt64Coin("eth", 0))
someEmptyCoins := sdk.Coins{sdk.NewInt64Coin("eth", 10), sdk.NewInt64Coin("atom", 0)}
unsortedCoins := sdk.Coins{sdk.NewInt64Coin("eth", 1), sdk.NewInt64Coin("atom", 1)}

cases := []struct {
valid bool
txOut Output
expectedErr string // empty means no error expected
txOut Output
}{
// auth works with different apps
{true, NewOutput(addr1, someCoins)},
{true, NewOutput(addr2, someCoins)},
{true, NewOutput(addr2, multiCoins)},

{false, NewOutput(emptyAddr, someCoins)}, // empty address
{false, NewOutput(addr1, emptyCoins)}, // invalid coins
{false, NewOutput(addr1, emptyCoins2)}, // invalid coins
{false, NewOutput(addr1, someEmptyCoins)}, // invalid coins
{false, NewOutput(addr1, unsortedCoins)}, // unsorted coins
{"", NewOutput(addr1, someCoins)},
{"", NewOutput(addr2, someCoins)},
{"", NewOutput(addr2, multiCoins)},

{"Invalid output address (incorrect address length (expected: 20, actual: 0)): invalid address", NewOutput(addrEmpty, someCoins)},
{"Invalid output address (incorrect address length (expected: 20, actual: 33)): invalid address", NewOutput(addrTooLong, someCoins)},
{": invalid coins", NewOutput(addr1, emptyCoins)}, // invalid coins
{": invalid coins", NewOutput(addr1, emptyCoins2)}, // invalid coins
{"10eth,0atom: invalid coins", NewOutput(addr1, someEmptyCoins)}, // invalid coins
{"1eth,1atom: invalid coins", NewOutput(addr1, unsortedCoins)}, // unsorted coins
}

for i, tc := range cases {
err := tc.txOut.ValidateBasic()
if tc.valid {
if tc.expectedErr == "" {
require.Nil(t, err, "%d: %+v", i, err)
} else {
require.NotNil(t, err, "%d", i)
require.EqualError(t, err, tc.expectedErr, "%d", i)
}
}
}

func TestMsgMultiSendValidation(t *testing.T) {
addr1 := sdk.AccAddress([]byte{1, 2})
addr2 := sdk.AccAddress([]byte{7, 8})
addr1 := sdk.AccAddress([]byte("_______alice________"))
addr2 := sdk.AccAddress([]byte("________bob_________"))
atom123 := sdk.NewCoins(sdk.NewInt64Coin("atom", 123))
atom124 := sdk.NewCoins(sdk.NewInt64Coin("atom", 124))
eth123 := sdk.NewCoins(sdk.NewInt64Coin("eth", 123))
Expand Down

0 comments on commit 574c7d2

Please sign in to comment.