From 56fa44662bffc0b1daf7f24bf907500634fca8a5 Mon Sep 17 00:00:00 2001 From: twothirtyfive Date: Thu, 14 Nov 2024 13:21:01 -0500 Subject: [PATCH 1/2] improvement: check recipient address length --- keeper/keeper.go | 9 +++++++-- keeper/msg_server.go | 4 ++++ keeper/query_server.go | 5 +++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/keeper/keeper.go b/keeper/keeper.go index 3ff98bf..de81b64 100644 --- a/keeper/keeper.go +++ b/keeper/keeper.go @@ -129,7 +129,7 @@ func (k *Keeper) ExecuteForwards(ctx context.Context) { } timeout := uint64(k.headerService.GetHeaderInfo(ctx).Time.UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp - _, err := k.transferKeeper.Transfer(ctx, &transfertypes.MsgTransfer{ + msg := &transfertypes.MsgTransfer{ SourcePort: transfertypes.PortID, SourceChannel: forward.Channel, Token: balance, @@ -138,7 +138,12 @@ func (k *Keeper) ExecuteForwards(ctx context.Context) { TimeoutHeight: clienttypes.ZeroHeight(), TimeoutTimestamp: timeout, Memo: "", - }) + } + if err := msg.ValidateBasic(); err != nil { + k.Logger().Error("message validation failed", "channel", forward.Channel, "address", forward.GetAddress().String(), "amount", balance.String(), "err", err) + continue + } + _, err := k.transferKeeper.Transfer(ctx, msg) if err != nil { // TODO: Consider moving to persistent store in order to retry in future blocks? k.Logger().Error("unable to execute automatic forward", "channel", forward.Channel, "address", forward.GetAddress().String(), "amount", balance.String(), "err", err) diff --git a/keeper/msg_server.go b/keeper/msg_server.go index 7ab2a40..2e2463f 100644 --- a/keeper/msg_server.go +++ b/keeper/msg_server.go @@ -20,6 +20,10 @@ func (k *Keeper) RegisterAccount(ctx context.Context, msg *types.MsgRegisterAcco return nil, errors.New("invalid channel") } + if len(msg.Recipient) > transfertypes.MaximumReceiverLength { + return nil, fmt.Errorf("recipient address must not exceed %d bytes", transfertypes.MaximumReceiverLength) + } + if msg.Fallback != "" { if _, err := k.accountKeeper.AddressCodec().StringToBytes(msg.Fallback); err != nil { return nil, errors.New("invalid fallback address") diff --git a/keeper/query_server.go b/keeper/query_server.go index 4e8f3ae..758f559 100644 --- a/keeper/query_server.go +++ b/keeper/query_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" @@ -27,6 +28,10 @@ func (k *Keeper) Address(ctx context.Context, req *types.QueryAddress) (*types.Q return nil, errorstypes.ErrInvalidRequest } + if len(req.Recipient) > transfertypes.MaximumReceiverLength { + return nil, fmt.Errorf("recipient address must not exceed %d bytes", transfertypes.MaximumReceiverLength) + } + if req.Fallback != "" { _, err := k.accountKeeper.AddressCodec().StringToBytes(req.Fallback) if err != nil { From 2298f2e77018a0c989bf8a4e812512745eab196a Mon Sep 17 00:00:00 2001 From: twothirtyfive Date: Wed, 20 Nov 2024 12:40:38 -0500 Subject: [PATCH 2/2] updated as per comments --- .changelog/unreleased/improvements/22-recipient-length-check.md | 2 ++ keeper/keeper.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 .changelog/unreleased/improvements/22-recipient-length-check.md diff --git a/.changelog/unreleased/improvements/22-recipient-length-check.md b/.changelog/unreleased/improvements/22-recipient-length-check.md new file mode 100644 index 0000000..14293d2 --- /dev/null +++ b/.changelog/unreleased/improvements/22-recipient-length-check.md @@ -0,0 +1,2 @@ +- Added check to ensure recipient addresses do not exceed IBC limit (2048 bytes) + ([\#22](https://github.com/noble-assets/forwarding/pull/22)) \ No newline at end of file diff --git a/keeper/keeper.go b/keeper/keeper.go index de81b64..faaa29e 100644 --- a/keeper/keeper.go +++ b/keeper/keeper.go @@ -140,7 +140,7 @@ func (k *Keeper) ExecuteForwards(ctx context.Context) { Memo: "", } if err := msg.ValidateBasic(); err != nil { - k.Logger().Error("message validation failed", "channel", forward.Channel, "address", forward.GetAddress().String(), "amount", balance.String(), "err", err) + k.Logger().Error("ibc message validation failed", "channel", forward.Channel, "address", forward.GetAddress().String(), "amount", balance.String(), "err", err) continue } _, err := k.transferKeeper.Transfer(ctx, msg)