(chore) disallow unknown fields29 fee ack (#6933) (#6942)

* (chore): disallow unknown fields in 29-fee ack

* Fixed comment

* Fix docstring

* Use json.Unmarshal rather than direct call

* Wrap the error in ibcerrors.ErrInvalidType

* Use json.Unmarshal in tests

(cherry picked from commit 8440245)

Co-authored-by: Nikolas De Giorgis <nikolas.degiorgis@interchain.io>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

modules/apps/29-fee/ibc_middleware.go

@@ -1,6 +1,7 @@
 package fee
 
 import (
+	"encoding/json"
 	"strings"
 
 	errorsmod "cosmossdk.io/errors"
@@ -13,6 +14,7 @@ import (
 	clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
+	ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
 	"github.com/cosmos/ibc-go/v9/modules/core/exported"
 )
 
@@ -250,8 +252,8 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 	}
 
 	var ack types.IncentivizedAcknowledgement
-	if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
-		return errorsmod.Wrapf(err, "cannot unmarshal ICS-29 incentivized packet acknowledgement: %v", ack)
+	if err := json.Unmarshal(acknowledgement, &ack); err != nil {
+		return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-29 incentivized packet acknowledgement %v: %s", ack, err)
 	}
 
 	if im.keeper.IsLocked(ctx) {

modules/apps/29-fee/ibc_middleware_test.go

@@ -1,6 +1,7 @@
 package fee_test
 
 import (
+	"encoding/json"
 	"fmt"
 
 	errorsmod "cosmossdk.io/errors"
@@ -1592,3 +1593,34 @@ func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterfaceError() {
 	expError := errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil))
 	suite.Require().ErrorIs(err, expError)
 }
+
+func (suite *FeeTestSuite) TestAckUnmarshal() {
+	testCases := []struct {
+		name     string
+		ackBytes []byte
+		expPass  bool
+	}{
+		{
+			"success",
+			[]byte(`{"app_acknowledgement": "eyJyZXN1bHQiOiJiVzlqYXlCaFkydHViM2RzWldsblpXMWxiblE9In0=", "forward_relayer_address": "relayer", "underlying_app_success": true}`),
+			true,
+		},
+		{
+			"failure: unknown fields",
+			[]byte(`{"app_acknowledgement": "eyJyZXN1bHQiOiJiVzlqYXlCaFkydHViM2RzWldsblpXMWxiblE9In0=", "forward_relayer_address": "relayer", "underlying_app_success": true, "extra_field": "foo"}`),
+			false,
+		},
+	}
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			ack := &types.IncentivizedAcknowledgement{}
+			err := json.Unmarshal(tc.ackBytes, ack)
+
+			if tc.expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().Error(err)
+			}
+		})
+	}
+}

modules/apps/29-fee/types/encoding.go

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