fix(transfer): Don't allow unknown fields during json unmarshalling. (#…

…6428) * fix(transfer): Don't allow unknown fields during json unmarshalling. Use json's DisallowUnknownFields flag on the decoder to control error emission for uknown fields. Implement Unmarshaller interface for both version of the ics20 packet data. * docs: expand on recursion protection.
cosmos · May 30, 2024 · 80b0c9a · 80b0c9a
1 parent 45c7eea
commit 80b0c9a
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 18 deletions.
diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go
@@ -1088,15 +1088,6 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 		expError    error
 		expAckError error
 	}{
-		{
-			"success: new field v2",
-			func() {
-				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"base": "atom", "trace": []},"amount":"100"}],"sender":"%s","receiver":"%s", "new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
-				packetData = []byte(jsonString)
-			},
-			nil,
-			nil,
-		},
 		{
 			"success: no new field with memo v2",
 			func() {
@@ -1124,24 +1115,22 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 			ibcerrors.ErrInvalidType,
 		},
 		{
-			"failure: missing field v2",
+			"failure: new field v2",
 			func() {
-				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"trace": []},"amount":"100"}],"sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
+				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"base": "atom", "trace": []},"amount":"100"}],"sender":"%s","receiver":"%s", "new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
 				packetData = []byte(jsonString)
 			},
-			types.ErrInvalidDenomForTransfer,
+			ibcerrors.ErrInvalidType,
 			ibcerrors.ErrInvalidType,
 		},
 		{
-			"success: new field",
+			"failure: missing field v2",
 			func() {
-				path.EndpointA.ChannelConfig.Version = types.V1
-				path.EndpointB.ChannelConfig.Version = types.V1
-				jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
+				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"trace": []},"amount":"100"}],"sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
 				packetData = []byte(jsonString)
 			},
-			nil,
-			nil,
+			types.ErrInvalidDenomForTransfer,
+			ibcerrors.ErrInvalidType,
 		},
 		{
 			"success: no new field with memo",
@@ -1175,6 +1164,17 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 			ibcerrors.ErrInvalidType,
 			ibcerrors.ErrInvalidType,
 		},
+		{
+			"failure: new field",
+			func() {
+				path.EndpointA.ChannelConfig.Version = types.V1
+				path.EndpointB.ChannelConfig.Version = types.V1
+				jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
+				packetData = []byte(jsonString)
+			},
+			ibcerrors.ErrInvalidType,
+			ibcerrors.ErrInvalidType,
+		},
 		{
 			"failure: missing field",
 			func() {

diff --git a/modules/apps/transfer/types/encoding.go b/modules/apps/transfer/types/encoding.go
@@ -0,0 +1,46 @@
+package types
+
+import (
+	"bytes"
+	"encoding/json"
+)
+
+// UnmarshalJSON implements the Unmarshaller interface for FungibleTokenPacketDataV2.
+func (ftpd *FungibleTokenPacketDataV2) UnmarshalJSON(bz []byte) error {
+	// Recursion protection. We cannot unmarshal into FungibleTokenPacketData directly
+	// else UnmarshalJSON is going to get invoked again, ad infinum. Create an alias
+	// and unmarshal into that, instead.
+	type ftpdAlias FungibleTokenPacketDataV2
+
+	d := json.NewDecoder(bytes.NewReader(bz))
+	// Raise errors during decoding if unknown fields are encountered.
+	d.DisallowUnknownFields()
+
+	var alias ftpdAlias
+	if err := d.Decode(&alias); err != nil {
+		return err
+	}
+
+	*ftpd = FungibleTokenPacketDataV2(alias)
+	return nil
+}
+
+// UnmarshalJSON implements the Unmarshaller interface for FungibleTokenPacketData.
+func (ftpd *FungibleTokenPacketData) UnmarshalJSON(bz []byte) error {
+	// Recursion protection. We cannot unmarshal into FungibleTokenPacketData directly
+	// else UnmarshalJSON is going to get invoked again, ad infinum. Create an alias
+	// and unmarshal into that, instead.
+	type ftpdAlias FungibleTokenPacketData
+
+	d := json.NewDecoder(bytes.NewReader(bz))
+	// Raise errors during decoding if unknown fields are encountered.
+	d.DisallowUnknownFields()
+
+	var alias ftpdAlias
+	if err := d.Decode(&alias); err != nil {
+		return err
+	}
+
+	*ftpd = FungibleTokenPacketData(alias)
+	return nil
+}