From 5213bbd65326edb9cd84f70c1bc0232b7389a4f7 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Sat, 14 Jan 2023 01:30:46 +0100 Subject: [PATCH] fix: fix allow list bypassed when whole spend limit used (#14615) --- x/bank/types/send_authorization.go | 11 +++++------ x/bank/types/send_authorization_test.go | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/x/bank/types/send_authorization.go b/x/bank/types/send_authorization.go index 2e8ee91e9e..81f5118f0f 100644 --- a/x/bank/types/send_authorization.go +++ b/x/bank/types/send_authorization.go @@ -33,19 +33,14 @@ func (a SendAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRes return authz.AcceptResponse{}, sdkerrors.ErrInvalidType.Wrap("type mismatch") } - toAddr := mSend.ToAddress - limitLeft, isNegative := a.SpendLimit.SafeSub(mSend.Amount...) if isNegative { return authz.AcceptResponse{}, sdkerrors.ErrInsufficientFunds.Wrapf("requested amount is more than spend limit") } - if limitLeft.IsZero() { - return authz.AcceptResponse{Accept: true, Delete: true}, nil - } isAddrExists := false + toAddr := mSend.ToAddress allowedList := a.GetAllowList() - for _, addr := range allowedList { ctx.GasMeter().ConsumeGas(gasCostPerIteration, "send authorization") if addr == toAddr { @@ -58,6 +53,10 @@ func (a SendAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRes return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrapf("cannot send to %s address", toAddr) } + if limitLeft.IsZero() { + return authz.AcceptResponse{Accept: true, Delete: true}, nil + } + return authz.AcceptResponse{Accept: true, Delete: false, Updated: &SendAuthorization{SpendLimit: limitLeft, AllowList: allowedList}}, nil } diff --git a/x/bank/types/send_authorization_test.go b/x/bank/types/send_authorization_test.go index 525ba5a14a..26cf064c2b 100644 --- a/x/bank/types/send_authorization_test.go +++ b/x/bank/types/send_authorization_test.go @@ -85,4 +85,25 @@ func TestSendAuthorization(t *testing.T) { require.NotNil(t, resp.Updated) // coins1000-coins500 = coins500 require.Equal(t, types.NewSendAuthorization(coins500, allowList).String(), resp.Updated.String()) + + t.Log("send everything to address not in allow list") + authzWithAllowList = types.NewSendAuthorization(coins1000, allowList) + require.Equal(t, authzWithAllowList.MsgTypeURL(), "/cosmos.bank.v1beta1.MsgSend") + require.NoError(t, authorization.ValidateBasic()) + send = types.NewMsgSend(fromAddr, unknownAddr, coins1000) + require.NoError(t, authzWithAllowList.ValidateBasic()) + resp, err = authzWithAllowList.Accept(ctx, send) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("cannot send to %s address", unknownAddr)) + + t.Log("send everything to address in allow list") + authzWithAllowList = types.NewSendAuthorization(coins1000, allowList) + require.Equal(t, authzWithAllowList.MsgTypeURL(), "/cosmos.bank.v1beta1.MsgSend") + require.NoError(t, authorization.ValidateBasic()) + send = types.NewMsgSend(fromAddr, allowList[0], coins1000) + require.NoError(t, authzWithAllowList.ValidateBasic()) + resp, err = authzWithAllowList.Accept(ctx, send) + require.NoError(t, err) + require.True(t, resp.Accept) + require.Nil(t, resp.Updated) }