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.

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 list of allowed packet data keys to Allocation of TransferAuthorization (backport #5280) #5486

Merged
merged 3 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (apps/transfer) [\#5280](https://github.com/cosmos/ibc-go/pull/5280) Add list of allowed packet data keys to `Allocation` of `TransferAuthorization`.

### Bug Fixes

## [v8.0.0](https://github.com/cosmos/ibc-go/releases/tag/v8.0.0) - 2023-11-10
Expand Down
6 changes: 6 additions & 0 deletions docs/docs/02-apps/01-transfer/08-authorizations.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ It takes:

- an `AllowList` list that specifies the list of addresses that are allowed to receive funds. If this list is empty, then all addresses are allowed to receive funds from the `TransferAuthorization`.

- an `AllowedPacketData` list that specifies the list of memo packet data keys that are allowed to send the packet. If this list is empty, then only an empty memo is allowed (a `memo` field with non-empty content will be denied). If this list includes a single element equal to `"*"`, then any content in `memo` field will be allowed.

Setting a `TransferAuthorization` is expected to fail if:

- the spend limit is nil
- the denomination of the spend limit is an invalid coin type
- the source port ID is invalid
- the source channel ID is invalid
- there are duplicate entries in the `AllowList`
- the `memo` field is not allowed by `AllowedPacketData`

Below is the `TransferAuthorization` message:

Expand All @@ -48,6 +51,9 @@ type Allocation struct {
SpendLimit sdk.Coins
// allow list of receivers, an empty allow list permits any receiver address
AllowList []string
// allow list of packet data keys, an empty list prohibits all packet data keys;
// a list only with "*" permits any packet data key
AllowedPacketData []string
}

```
113 changes: 86 additions & 27 deletions modules/apps/transfer/types/authz.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions modules/apps/transfer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ const (
// DenomPrefix is the prefix used for internal SDK coin representation.
DenomPrefix = "ibc"

// AllowAllPacketDataKeys holds the string key that allows all packet data keys in authz transfer messages
AllowAllPacketDataKeys = "*"

KeyTotalEscrowPrefix = "totalEscrowForDenom"

ParamsKey = "params"
Expand Down
64 changes: 60 additions & 4 deletions modules/apps/transfer/types/transfer_authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package types

import (
"context"
"encoding/json"
"math/big"
"sort"
"strings"

"github.com/cosmos/gogoproto/proto"

Expand Down Expand Up @@ -50,6 +53,11 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a
return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer")
}

err := validateMemo(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData)
if err != nil {
return authz.AcceptResponse{}, err
}

// If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit.
if allocation.SpendLimit.AmountOf(msgTransfer.Token.Denom).Equal(UnboundedSpendLimit()) {
return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil
Expand All @@ -70,10 +78,11 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a
}}, nil
}
a.Allocations[index] = Allocation{
SourcePort: allocation.SourcePort,
SourceChannel: allocation.SourceChannel,
SpendLimit: limitLeft,
AllowList: allocation.AllowList,
SourcePort: allocation.SourcePort,
SourceChannel: allocation.SourceChannel,
SpendLimit: limitLeft,
AllowList: allocation.AllowList,
AllowedPacketData: allocation.AllowedPacketData,
}

return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{
Expand Down Expand Up @@ -145,6 +154,53 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b
return false
}

// validateMemo returns a nil error indicating if the memo is valid for transfer.
func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) error {
// if the allow list is empty, then the memo must be an empty string
if len(allowedPacketDataList) == 0 {
if len(strings.TrimSpace(memo)) != 0 {
return errorsmod.Wrapf(ErrInvalidAuthorization, "memo must be empty because allowed packet data in allocation is empty")
}

return nil
}

// if allowedPacketDataList has only 1 element and it equals AllowAllPacketDataKeys
// then accept all the packet data keys
if len(allowedPacketDataList) == 1 && allowedPacketDataList[0] == AllowAllPacketDataKeys {
return nil
}

jsonObject := make(map[string]interface{})
err := json.Unmarshal([]byte(memo), &jsonObject)
if err != nil {
return err
}

gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat

for _, key := range allowedPacketDataList {
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization")

_, ok := jsonObject[key]
if ok {
delete(jsonObject, key)
}
}

var keys []string
for k := range jsonObject {
keys = append(keys, k)
}
sort.Strings(keys)

if len(jsonObject) != 0 {
return errorsmod.Wrapf(ErrInvalidAuthorization, "not allowed packet data keys: %s", keys)
}

return nil
}

// UnboundedSpendLimit returns the sentinel value that can be used
// as the amount for a denomination's spend limit for which spend limit updating
// should be disabled. Please note that using this sentinel value means that a grantee
Expand Down
69 changes: 69 additions & 0 deletions modules/apps/transfer/types/transfer_authorization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/cosmos/ibc-go/v8/testing/mock"
)

const testMemo = `{"wasm":{"contract":"osmo1c3ljch9dfw5kf52nfwpxd2zmj2ese7agnx0p9tenkrryasrle5sqf3ftpg","msg":{"osmosis_swap":{"output_denom":"uosmo","slippage":{"twap":{"slippage_percentage":"20","window_seconds":10}},"receiver":"feeabs/feeabs1efd63aw40lxf3n4mhf7dzhjkr453axurwrhrrw","on_failed_delivery":"do_nothing"}}}}`

func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
var (
msgTransfer types.MsgTransfer
Expand Down Expand Up @@ -101,6 +103,73 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
suite.Require().Nil(res.Updated)
},
},
{
"success: empty AllowedPacketData and empty memo",
func() {
allowedList := []string{}
transferAuthz.Allocations[0].AllowedPacketData = allowedList
},
func(res authz.AcceptResponse, err error) {
suite.Require().NoError(err)

suite.Require().True(res.Accept)
suite.Require().True(res.Delete)
suite.Require().Nil(res.Updated)
},
},
{
"success: AllowedPacketData allows any packet",
func() {
allowedList := []string{"*"}
transferAuthz.Allocations[0].AllowedPacketData = allowedList
msgTransfer.Memo = testMemo
},
func(res authz.AcceptResponse, err error) {
suite.Require().NoError(err)

suite.Require().True(res.Accept)
suite.Require().True(res.Delete)
suite.Require().Nil(res.Updated)
},
},
{
"success: transfer memo allowed",
func() {
allowedList := []string{"wasm", "forward"}
transferAuthz.Allocations[0].AllowedPacketData = allowedList
msgTransfer.Memo = testMemo
},
func(res authz.AcceptResponse, err error) {
suite.Require().NoError(err)

suite.Require().True(res.Accept)
suite.Require().True(res.Delete)
suite.Require().Nil(res.Updated)
},
},
{
"empty AllowedPacketData but not empty memo",
func() {
allowedList := []string{}
transferAuthz.Allocations[0].AllowedPacketData = allowedList
msgTransfer.Memo = testMemo
},
func(res authz.AcceptResponse, err error) {
suite.Require().Error(err)
},
},
{
"memo not allowed",
func() {
allowedList := []string{"forward"}
transferAuthz.Allocations[0].AllowedPacketData = allowedList
msgTransfer.Memo = testMemo
},
func(res authz.AcceptResponse, err error) {
suite.Require().Error(err)
suite.Require().ErrorContains(err, "not allowed packet data keys: [wasm]")
},
},
{
"test multiple coins does not overspend",
func() {
Expand Down
3 changes: 3 additions & 0 deletions proto/ibc/applications/transfer/v1/authz.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ message Allocation {
[(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
// allow list of receivers, an empty allow list permits any receiver address
repeated string allow_list = 4;
// allow list of packet data keys, an empty list prohibits all packet data keys;
// a list only with "*" permits any packet data key
repeated string allowed_packet_data = 5;
}

// TransferAuthorization allows the grantee to spend up to spend_limit coins from
Expand Down
Loading