diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go index 7cec17d38da..54f07bee76c 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go @@ -10,6 +10,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" + clienttypes "github.com/cosmos/ibc-go/v2/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v2/modules/core/24-host" ibctesting "github.com/cosmos/ibc-go/v2/testing" @@ -189,6 +190,54 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { } } +// Test initiating a ChanOpenTry using the controller chain instead of the host chain +// ChainA is the controller chain. ChainB creates a controller port as well, +// attempting to trick chainA. +// Sending a MsgChanOpenTry will never reach the application callback due to +// core IBC checks not passing, so a call to the application callback is also +// done directly. +func (suite *InterchainAccountsTestSuite) TestChanOpenTry() { + suite.SetupTest() // reset + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) + suite.Require().NoError(err) + + // chainB also creates a controller port + err = InitInterchainAccount(path.EndpointB, TestOwnerAddress) + suite.Require().NoError(err) + + path.EndpointA.UpdateClient() + channelKey := host.ChannelKey(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + proofInit, proofHeight := path.EndpointB.Chain.QueryProof(channelKey) + + // use chainA (controller) for ChanOpenTry + msg := channeltypes.NewMsgChannelOpenTry(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, TestVersion, channeltypes.ORDERED, []string{path.EndpointA.ConnectionID}, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, types.VersionPrefix, proofInit, proofHeight, types.ModuleName) + handler := suite.chainA.GetSimApp().MsgServiceRouter().Handler(msg) + _, err = handler(suite.chainA.GetContext(), msg) + + suite.Require().Error(err) + + // call application callback directly + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointB.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + chanCap, found := suite.chainA.App.GetScopedIBCKeeper().GetCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + suite.Require().True(found) + + err = cbs.OnChanOpenTry( + suite.chainA.GetContext(), path.EndpointA.ChannelConfig.Order, []string{path.EndpointA.ConnectionID}, + path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, + counterparty, path.EndpointA.ChannelConfig.Version, path.EndpointB.ChannelConfig.Version, + ) + suite.Require().Error(err) +} + func (suite *InterchainAccountsTestSuite) TestOnChanOpenAck() { var ( path *ibctesting.Path @@ -253,3 +302,230 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenAck() { } } + +// Test initiating a ChanOpenConfirm using the controller chain instead of the host chain +// ChainA is the controller chain. ChainB is the host chain +// Sending a MsgChanOpenConfirm will never reach the application callback due to +// core IBC checks not passing, so a call to the application callback is also +// done directly. +func (suite *InterchainAccountsTestSuite) TestChanOpenConfirm() { + suite.SetupTest() // reset + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) + suite.Require().NoError(err) + + err = path.EndpointB.ChanOpenTry() + suite.Require().NoError(err) + + // chainB maliciously sets channel to OPEN + channel := channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), []string{path.EndpointB.ConnectionID}, TestVersion) + suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.SetChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, channel) + + // commit state changes so proof can be created + suite.chainB.App.Commit() + suite.chainB.NextBlock() + + path.EndpointA.UpdateClient() + + // query proof from ChainB + channelKey := host.ChannelKey(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + proofAck, proofHeight := path.EndpointB.Chain.QueryProof(channelKey) + + // use chainA (controller) for ChanOpenConfirm + msg := channeltypes.NewMsgChannelOpenConfirm(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, proofAck, proofHeight, types.ModuleName) + handler := suite.chainA.GetSimApp().MsgServiceRouter().Handler(msg) + _, err = handler(suite.chainA.GetContext(), msg) + + suite.Require().Error(err) + + // call application callback directly + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanOpenConfirm( + suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, + ) + suite.Require().Error(err) + +} + +// OnChanCloseInit on controller (chainA) +func (suite *InterchainAccountsTestSuite) TestOnChanCloseInit() { + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanCloseInit( + suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, + ) + + suite.Require().Error(err) +} + +func (suite *InterchainAccountsTestSuite) TestOnChanCloseConfirm() { + var ( + path *ibctesting.Path + ) + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + + { + "success", func() {}, true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path = NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + tc.malleate() // malleate mutates test data + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanCloseConfirm( + suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + + activeChannelID, found := suite.chainA.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + + if tc.expPass { + suite.Require().NoError(err) + suite.Require().False(found) + suite.Require().Empty(activeChannelID) + } else { + suite.Require().Error(err) + } + + }) + } +} + +func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "ICA OnRecvPacket fails with ErrInvalidChannelFlow", func() {}, false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + tc.malleate() // malleate mutates test data + + module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + packet := channeltypes.NewPacket( + []byte("empty packet data"), + suite.chainA.SenderAccount.GetSequence(), + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + clienttypes.NewHeight(0, 100), + 0, + ) + + ack := cbs.OnRecvPacket(suite.chainA.GetContext(), packet, TestAccAddress) + suite.Require().Equal(tc.expPass, ack.Success()) + }) + } +} + +func (suite *InterchainAccountsTestSuite) TestNegotiateAppVersion() { + var ( + proposedVersion string + ) + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "ICA OnRecvPacket fails with ErrInvalidChannelFlow", func() {}, false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) + suite.Require().NoError(err) + + module, _, err := suite.chainA.GetSimApp().GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.GetSimApp().GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + counterpartyPortID, err := types.GeneratePortID(TestOwnerAddress, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + suite.Require().NoError(err) + + counterparty := channeltypes.Counterparty{ + PortId: counterpartyPortID, + ChannelId: path.EndpointB.ChannelID, + } + + proposedVersion = types.VersionPrefix + + tc.malleate() + + version, err := cbs.NegotiateAppVersion(suite.chainA.GetContext(), channeltypes.ORDERED, path.EndpointA.ConnectionID, path.EndpointA.ChannelConfig.PortID, counterparty, proposedVersion) + if tc.expPass { + suite.Require().NoError(err) + suite.Require().NoError(types.ValidateVersion(version)) + suite.Require().Equal(TestVersion, version) + } else { + suite.Require().Error(err) + suite.Require().Empty(version) + } + }) + } +} diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 6145c867109..f88f8f1fadc 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -70,6 +70,10 @@ func (k Keeper) OnChanOpenAck( channelID string, counterpartyVersion string, ) error { + if portID == types.PortID { + return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "portID cannot be host chain port ID: %s", types.PortID) + } + if err := types.ValidateVersion(counterpartyVersion); err != nil { return sdkerrors.Wrap(err, "counterparty version validation failed") } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index a4bd9622eab..f0e634fd362 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -174,6 +174,12 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { }, false, }, + { + "invalid portID", func() { + path.EndpointA.ChannelConfig.PortID = types.PortID + expectedChannelID = "" + }, false, + }, } for _, tc := range testCases { diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 92203c25ea8..b3362d9faaa 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -45,7 +45,7 @@ func TestICATestSuite(t *testing.T) { } func (suite *InterchainAccountsTestSuite) SetupTest() { - suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3) + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2) suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0)) suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) } @@ -106,6 +106,21 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { return nil } +// Test initiating a ChanOpenInit using the host chain instead of the controller chain +// ChainA is the controller chain. ChainB is the host chain +func (suite *InterchainAccountsTestSuite) TestChanOpenInit() { + suite.SetupTest() // reset + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + // use chainB (host) for ChanOpenInit + msg := channeltypes.NewMsgChannelOpenInit(path.EndpointB.ChannelConfig.PortID, types.VersionPrefix, channeltypes.ORDERED, []string{path.EndpointB.ConnectionID}, path.EndpointA.ChannelConfig.PortID, types.ModuleName) + handler := suite.chainB.GetSimApp().MsgServiceRouter().Handler(msg) + _, err := handler(suite.chainB.GetContext(), msg) + + suite.Require().Error(err) +} + func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { var ( path *ibctesting.Path @@ -192,6 +207,41 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { } +// Test initiating a ChanOpenAck using the host chain instead of the controller chain +// ChainA is the controller chain. ChainB is the host chain +func (suite *InterchainAccountsTestSuite) TestChanOpenAck() { + suite.SetupTest() // reset + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) + suite.Require().NoError(err) + + err = path.EndpointB.ChanOpenTry() + suite.Require().NoError(err) + + // chainA maliciously sets channel to TRYOPEN + channel := channeltypes.NewChannel(channeltypes.TRYOPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{path.EndpointA.ConnectionID}, TestVersion) + suite.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel) + + // commit state changes so proof can be created + suite.chainA.App.Commit() + suite.chainA.NextBlock() + + path.EndpointB.UpdateClient() + + // query proof from ChainA + channelKey := host.ChannelKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + proofTry, proofHeight := path.EndpointA.Chain.QueryProof(channelKey) + + // use chainB (host) for ChanOpenAck + msg := channeltypes.NewMsgChannelOpenAck(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelID, TestVersion, proofTry, proofHeight, types.ModuleName) + handler := suite.chainB.GetSimApp().MsgServiceRouter().Handler(msg) + _, err = handler(suite.chainB.GetContext(), msg) + + suite.Require().Error(err) +} + func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() { testCases := []struct { name string @@ -253,6 +303,77 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() { } +// OnChanCloseInit on host (chainB) +func (suite *InterchainAccountsTestSuite) TestOnChanCloseInit() { + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanCloseInit( + suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, + ) + + suite.Require().Error(err) +} + +func (suite *InterchainAccountsTestSuite) TestOnChanCloseConfirm() { + var ( + path *ibctesting.Path + ) + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + + { + "success", func() {}, true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path = NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + tc.malleate() // malleate mutates test data + module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanCloseConfirm( + suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + + activeChannelID, found := suite.chainB.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) + + if tc.expPass { + suite.Require().NoError(err) + suite.Require().False(found) + suite.Require().Empty(activeChannelID) + } else { + suite.Require().Error(err) + } + + }) + } +} + func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { var ( packetData []byte @@ -337,6 +458,114 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { } +func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() { + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "ICA OnAcknowledgementPacket fails with ErrInvalidChannelFlow", func() {}, false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + tc.malleate() // malleate mutates test data + + module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + packet := channeltypes.NewPacket( + []byte("empty packet data"), + suite.chainA.SenderAccount.GetSequence(), + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + clienttypes.NewHeight(0, 100), + 0, + ) + + err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), packet, []byte("ackBytes"), TestAccAddress) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } +} + +func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() { + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "ICA OnTimeoutPacket fails with ErrInvalidChannelFlow", func() {}, false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + tc.malleate() // malleate mutates test data + + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointB.ChannelConfig.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + packet := channeltypes.NewPacket( + []byte("empty packet data"), + suite.chainA.SenderAccount.GetSequence(), + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + clienttypes.NewHeight(0, 100), + 0, + ) + + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, TestAccAddress) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } +} + func (suite *InterchainAccountsTestSuite) TestNegotiateAppVersion() { var ( proposedVersion string