From f7bf4a3708d10f5842bf60fe7db5e4ea5da43732 Mon Sep 17 00:00:00 2001
From: Gjermund Garaba <gjermund@garaba.net>
Date: Thu, 13 Jun 2024 14:25:29 +0200
Subject: [PATCH] feat(transfer): validate forwarding memo in transfer
 authorization

---
 .../transfer/types/transfer_authorization.go  |  9 ++-
 .../types/transfer_authorization_test.go      | 68 +++++++++++++++++++
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go
index 01bcafb48c6..8d8ca7dbcaf 100644
--- a/modules/apps/transfer/types/transfer_authorization.go
+++ b/modules/apps/transfer/types/transfer_authorization.go
@@ -58,8 +58,13 @@ func (a TransferAuthorization) Accept(goCtx context.Context, msg proto.Message)
 		return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer")
 	}
 
-	err := validateMemo(ctx, msgTransfer.Memo, a.Allocations[index].AllowedPacketData)
-	if err != nil {
+	memo := msgTransfer.Memo
+	// in the case of forwarded transfers, the actual memo is stored in the forwarding path until the final destination
+	if msgTransfer.Forwarding != nil && len(msgTransfer.Forwarding.Hops) > 0 {
+		memo = msgTransfer.Forwarding.Memo
+	}
+
+	if err := validateMemo(ctx, memo, a.Allocations[index].AllowedPacketData); err != nil {
 		return authz.AcceptResponse{}, err
 	}
 
diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go
index 8aee601173c..718d299aac5 100644
--- a/modules/apps/transfer/types/transfer_authorization_test.go
+++ b/modules/apps/transfer/types/transfer_authorization_test.go
@@ -99,6 +99,21 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 				suite.Require().Nil(res.Updated)
 			},
 		},
+		{
+			"success: empty AllowedPacketData and empty memo in forwarding path",
+			func() {
+				allowedList := []string{}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Forwarding = types.NewForwarding("", validHop)
+			},
+			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() {
@@ -114,6 +129,21 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 				suite.Require().Nil(res.Updated)
 			},
 		},
+		{
+			"success: AllowedPacketData allows any packet in forwarding path",
+			func() {
+				allowedList := []string{"*"}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Forwarding = types.NewForwarding(testMemo1, validHop)
+			},
+			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() {
@@ -129,6 +159,21 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 				suite.Require().Nil(res.Updated)
 			},
 		},
+		{
+			"success: transfer memo allowed in forwarding path",
+			func() {
+				allowedList := []string{testMemo1, testMemo2}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Forwarding = types.NewForwarding(testMemo1, validHop)
+			},
+			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() {
@@ -140,6 +185,17 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 				suite.Require().Error(err)
 			},
 		},
+		{
+			"empty AllowedPacketData but not empty memo in forwarding path",
+			func() {
+				allowedList := []string{}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Forwarding = types.NewForwarding(testMemo1, validHop)
+			},
+			func(res authz.AcceptResponse, err error) {
+				suite.Require().Error(err)
+			},
+		},
 		{
 			"memo not allowed",
 			func() {
@@ -152,6 +208,18 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 				suite.Require().ErrorContains(err, fmt.Sprintf("not allowed memo: %s", testMemo2))
 			},
 		},
+		{
+			"memo not allowed in forwarding path",
+			func() {
+				allowedList := []string{testMemo1}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Forwarding = types.NewForwarding(testMemo2, validHop)
+			},
+			func(res authz.AcceptResponse, err error) {
+				suite.Require().Error(err)
+				suite.Require().ErrorContains(err, fmt.Sprintf("not allowed memo: %s", testMemo2))
+			},
+		},
 		{
 			"test multiple coins does not overspend",
 			func() {