Skip to content

Commit

Permalink
x/bank/types: fix AddressFromBalancesStore address length overflow (#…
Browse files Browse the repository at this point in the history
…9112)

addrLen is encoded in a byte, so it's an uint8. The code in
AddressFromBalancesStore cast it to int for bound checking, but wrongly uses "addrLen+1", which can be overflow.

To fix this, just cast addrLen once and use it in all places.

Found by fuzzing added in #9060.

Fixes #9111
  • Loading branch information
Cuong Manh Le committed Apr 15, 2021
1 parent a4a6e05 commit ef69863
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
7 changes: 3 additions & 4 deletions x/bank/types/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) {
return nil, ErrInvalidKey
}
addrLen := key[0]
if len(key[1:]) < int(addrLen) {
bound := int(addrLen)
if len(key)-1 < bound {
return nil, ErrInvalidKey
}
addr := key[1 : addrLen+1]

return sdk.AccAddress(addr), nil
return key[1 : bound+1], nil
}

// CreateAccountBalancesPrefix creates the prefix for an account's balances.
Expand Down
31 changes: 18 additions & 13 deletions x/bank/types/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,34 @@ func TestAddressFromBalancesStore(t *testing.T) {
require.NoError(t, err)
addrLen := len(addr)
require.Equal(t, 20, addrLen)

key := cloneAppend(address.MustLengthPrefix(addr), []byte("stake"))
res, err := types.AddressFromBalancesStore(key)
require.NoError(t, err)
require.Equal(t, res, addr)
}

func TestInvalidAddressFromBalancesStore(t *testing.T) {
tests := []struct {
name string
key []byte
name string
key []byte
wantErr bool
expectedKey sdk.AccAddress
}{
{"empty", []byte("")},
{"invalid", []byte("3AA")},
{"valid", key, false, addr},
{"#9111", []byte("\xff000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"), false, nil},
{"empty", []byte(""), true, nil},
{"invalid", []byte("3AA"), true, nil},
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
_, err := types.AddressFromBalancesStore(tc.key)
assert.Error(t, err)
assert.True(t, errors.Is(types.ErrInvalidKey, err))
addr, err := types.AddressFromBalancesStore(tc.key)
if tc.wantErr {
assert.Error(t, err)
assert.True(t, errors.Is(types.ErrInvalidKey, err))
} else {
assert.NoError(t, err)
}
if len(tc.expectedKey) > 0 {
assert.Equal(t, tc.expectedKey, addr)
}
})
}
}

0 comments on commit ef69863

Please sign in to comment.