Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add address length validation to MsgSend and MsgMultiSend #7053

Merged
merged 9 commits into from
Aug 17, 2020
Merged

Add address length validation to MsgSend and MsgMultiSend #7053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
17feded
Test MsgSend errors
webmaster128 Aug 14, 2020
60a517b
Improve length check in MsgSend.ValidateBasic
webmaster128 Aug 14, 2020
db4a503
Add expected errors to TestInputValidation
webmaster128 Aug 14, 2020
3342b51
Test address length for Input
webmaster128 Aug 14, 2020
7b30a47
Test error messages in TestOutputValidation
webmaster128 Aug 14, 2020
331f0d1
Validate address lentgth in Output
webmaster128 Aug 14, 2020
363e576
Merge branch 'master' into MsgSend-ValidateBasic
webmaster128 Aug 17, 2020
178ecc3
Merge branch 'master' into MsgSend-ValidateBasic
alexanderbez Aug 17, 2020
05f0e4f
Merge branch 'master' into MsgSend-ValidateBasic
alexanderbez Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions x/bank/keeper/keeper_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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