fix(cronos): add contract authorization check to SendCroToIbcHandler

CHANGELOG.md

@@ -28,6 +28,8 @@
 * [#2070](https://github.com/crypto-org-chain/cronos/pull/2070) fix(ci): fix github pr labeler to match v6 format.
 * [#2069](https://github.com/crypto-org-chain/cronos/pull/2069) fix(test): flaky test_mempool integration test.
 * [#2071](https://github.com/crypto-org-chain/cronos/pull/2071) fix(test): fix flaky ibc and versiondb integration tests.
+* [#2099](https://github.com/crypto-org-chain/cronos/pull/2099) fix(cronos): add contract authorization check to SendCroToIbcHandler
+
 
 ### Chores:
 * [#1986](https://github.com/crypto-org-chain/cronos/pull/1986) Remove unused precompiles

app/upgrades.go

@@ -8,6 +8,7 @@ import (
 
 	host "github.com/cosmos/ibc-go/v11/modules/core/24-host"
 	ibcexported "github.com/cosmos/ibc-go/v11/modules/core/exported"
+	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/runtime"
@@ -19,11 +20,23 @@ import (
 
 const planName = "v1.8"
 
+// croBridgeContractAddresses are the EVM addresses of CroBridge contracts
+// authorized on Cronos mainnet. Empty list disables the SendCroToIbc hook.
+var croBridgeContractAddresses = []string{
+	"0x6b1b50c2223eb31E0d4683b046ea9C6CB0D0ea4F",
+	"0xCE13a6F3d4167CE958f4764D423e6D62a114c751",
+}
+
 // RegisterUpgradeHandlers returns if store loader is overridden.
 // No store-key churn from v0.53→v0.54 in this app, so the default
 // MaxVersionStoreLoader (set by the caller when this returns false)
 // covers both regular and upgrade-height boots.
 func (app *App) RegisterUpgradeHandlers(cdc codec.BinaryCodec, maxVersion int64) bool {
+	for _, addr := range croBridgeContractAddresses {
+		if !common.IsHexAddress(addr) || (common.HexToAddress(addr) == common.Address{}) {
+			panic(fmt.Sprintf("invalid croBridgeContractAddresses entry %q: must be a non-zero EVM hex address", addr))
+		}
+	}
 	app.UpgradeKeeper.SetUpgradeHandler(planName,
 		func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
 			toVM, err := app.ModuleManager.RunMigrations(ctx, app.configurator, fromVM)
@@ -50,6 +63,14 @@ func (app *App) RegisterUpgradeHandlers(cdc codec.BinaryCodec, maxVersion int64)
 			)); err != nil {
 				return toVM, fmt.Errorf("prune stale ibc consensus state subkeys: %w", err)
 			}
+			// Set CroBridgeContractAddresses to authorize the canonical CroBridge contracts
+			// for the SendCroToIbc hook. This closes the unauthenticated-drain vulnerability
+			// where any contract emitting __CronosSendCroToIbc could drain CRO balances.
+			cronosParams := app.CronosKeeper.GetParams(sdkCtx)
+			cronosParams.CroBridgeContractAddresses = croBridgeContractAddresses
+			if err := app.CronosKeeper.SetParams(sdkCtx, cronosParams); err != nil {
+				return toVM, fmt.Errorf("set cro bridge contract addresses: %w", err)
+			}
 			return toVM, nil
 		},
 	)

app/upgrades_test.go

@@ -6,6 +6,7 @@ import (
 
 	dbm "github.com/cosmos/cosmos-db"
 	host "github.com/cosmos/ibc-go/v11/modules/core/24-host"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/stretchr/testify/require"
 
 	"cosmossdk.io/log/v2"
@@ -60,3 +61,30 @@ func TestPruneStaleIBCConsensusStateSubkeys(t *testing.T) {
 		require.NotNil(t, store.Get([]byte(k)), "canonical key %s must survive second run", k)
 	}
 }
+
+// TestUpgradeV18CroBridgeContractAddresses verifies that:
+//  1. croBridgeContractAddresses are all valid EVM addresses.
+//  2. The upgrade handler param migration correctly persists the values via SetParams/GetParams.
+func TestUpgradeV18CroBridgeContractAddresses(t *testing.T) {
+	for _, addr := range croBridgeContractAddresses {
+		require.True(t, common.IsHexAddress(addr),
+			"croBridgeContractAddresses entry must be a valid EVM hex address, got: %s",
+			addr)
+		require.NotEqual(t, common.Address{}, common.HexToAddress(addr),
+			"croBridgeContractAddresses entry must not be the zero address, got: %s",
+			addr)
+	}
+
+	// Verify the migration code path: GetParams → mutate → SetParams → GetParams round-trips correctly.
+	a := Setup(t, "")
+	ctx := a.NewContext(false)
+
+	// Apply the same mutation the upgrade handler performs.
+	params := a.CronosKeeper.GetParams(ctx)
+	params.CroBridgeContractAddresses = croBridgeContractAddresses
+	require.NoError(t, a.CronosKeeper.SetParams(ctx, params))
+
+	stored := a.CronosKeeper.GetParams(ctx)
+	require.ElementsMatch(t, croBridgeContractAddresses, stored.CroBridgeContractAddresses,
+		"CroBridgeContractAddresses not persisted by SetParams")
+}

integration_tests/test_gov_update_params.py

@@ -52,6 +52,7 @@ def test_evm_update_param(cronos, tmp_path):
 def test_gov_update_params(cronos):
     params = {
         "cronos_admin": "crc12luku6uxehhak02py4rcz65zu0swh7wjsrw0pp",
+        "cro_bridge_contract_addresses": [],
         "enable_auto_deployment": False,
         "ibc_cro_denom": "ibc/6411AE2ADA1E73DB59DB151"
         "A8988F9B7D5E7E233D8414DB6817F8F1A01600000",

integration_tests/test_ibc.py

@@ -20,6 +20,7 @@
     deploy_contract,
     parse_events_rpc,
     send_transaction,
+    submit_gov_proposal,
     wait_for_fn,
 )
 
@@ -186,6 +187,35 @@ def test_cro_bridge_contract(ibc):
     # case 2: use CroBridge contract
     w3 = ibc.cronos.w3
     contract = deploy_contract(w3, CONTRACTS["CroBridge"])
+
+    # Authorize the deployed CroBridge contract via governance before making
+    # the bridge call. Without this, SendCroToIbcHandler rejects all callers.
+    cli = ibc.cronos.cosmos_cli()
+    gov_params = cli.query_params()
+    gov_params["cro_bridge_contract_addresses"] = [contract.address]
+    update_msg = "/cronos.MsgUpdateParams"
+    submit_gov_proposal(
+        ibc.cronos,
+        update_msg,
+        messages=[
+            {
+                "@type": update_msg,
+                "authority": module_address("gov"),
+                "params": gov_params,
+            }
+        ],
+    )
+
+    # Verify the param was stored correctly
+    stored_params = cli.query_params()
+    stored_addresses = [
+        a.lower() for a in stored_params.get("cro_bridge_contract_addresses", [])
+    ]
+    assert contract.address.lower() in stored_addresses, (
+        f"cro_bridge_contract_addresses does not contain "
+        f"{contract.address}: {stored_params}"
+    )
+
     tx = contract.functions.send_cro_to_crypto_org(dst_addr).build_transaction(
         {
             "from": ADDRS["signer2"],

integration_tests/test_upgrade.py

@@ -376,6 +376,13 @@ def do_upgrade(plan_name, target, mode=None):
     check_basic_tx(c)
     check_ibc_client_states(ports.api_port(base_port), ibc_snapshot)
 
+    # Verify CroBridgeContractAddresses param was written by the upgrade handler.
+    cronos_params = cli.query_params()
+    assert cronos_params.get("cro_bridge_contract_addresses", []) == [
+        "0x6b1b50c2223eb31E0d4683b046ea9C6CB0D0ea4F",
+        "0xCE13a6F3d4167CE958f4764D423e6D62a114c751",
+    ], f"unexpected cro_bridge_contract_addresses: {cronos_params}"
+
     tx_af = w3.provider.make_request(method, params)
     assert tx_af.get("result") == tx_bf.get("result"), tx_af
 

proto/cronos/cronos.proto

@@ -14,6 +14,8 @@ message Params {
   string cronos_admin           = 3;
   bool   enable_auto_deployment = 4;
   uint64 max_callback_gas       = 5;
+  // the authorized contract addresses for the SendCroToIbc hook; empty list disables the hook
+  repeated string cro_bridge_contract_addresses = 6;
 }
 
 // TokenMappingChangeProposal defines a proposal to change one token mapping.

x/cronos/keeper/evmhandlers/send_cro_to_ibc.go

@@ -1,7 +1,9 @@
 package evmhandler
 
 import (
+	"fmt"
 	"math/big"
+	"slices"
 
 	cronoskeeper "github.com/crypto-org-chain/cronos/x/cronos/keeper"
 	"github.com/crypto-org-chain/cronos/x/cronos/types"
@@ -70,6 +72,13 @@ func (h SendCroToIbcHandler) Handle(
 	data []byte,
 	_ func(contractAddress common.Address, logSig common.Hash, logData []byte),
 ) error {
+	authorizedBridges := h.cronosKeeper.GetParams(ctx).CroBridgeContractAddresses
+	if !slices.ContainsFunc(authorizedBridges, func(addr string) bool {
+		return common.HexToAddress(addr) == contract
+	}) {
+		return fmt.Errorf("contract %s is not authorized to use SendCroToIbc hook", contract)
+	}
+
 	unpacked, err := SendCroToIbcEvent.Inputs.Unpack(data)
 	if err != nil {
 		// log and ignore

x/cronos/keeper/evmhandlers_test.go

@@ -356,9 +356,62 @@ func (suite *KeeperTestSuite) TestSendCroToIbcHandler() {
 		postcheck func()
 		error     error
 	}{
+		{
+			"unauthorized contract (param not set), expect fail",
+			func() {
+				params := suite.app.CronosKeeper.GetParams(suite.ctx)
+				params.CroBridgeContractAddresses = []string{}
+				err := suite.app.CronosKeeper.SetParams(suite.ctx, params)
+				suite.Require().NoError(err)
+
+				coin := sdk.NewCoin(suite.evmParam.EvmDenom, sdkmath.NewInt(1230000000500))
+				err = suite.MintCoins(contract.Bytes(), sdk.NewCoins(coin))
+				suite.Require().NoError(err)
+				topics = []common.Hash{
+					evmhandlers.SendCroToIbcEvent.ID,
+				}
+				input, _ := evmhandlers.SendToIbcEvent.Inputs.NonIndexed().Pack(
+					sender,
+					"recipient",
+					coin.Amount.BigInt(),
+				)
+				data = input
+			},
+			func() {},
+			fmt.Errorf("contract %s is not authorized to use SendCroToIbc hook", contract),
+		},
+		{
+			"unauthorized contract (different address in param), expect fail",
+			func() {
+				params := suite.app.CronosKeeper.GetParams(suite.ctx)
+				params.CroBridgeContractAddresses = []string{common.BigToAddress(big.NewInt(0x999)).Hex()}
+				err := suite.app.CronosKeeper.SetParams(suite.ctx, params)
+				suite.Require().NoError(err)
+
+				coin := sdk.NewCoin(suite.evmParam.EvmDenom, sdkmath.NewInt(1230000000500))
+				err = suite.MintCoins(contract.Bytes(), sdk.NewCoins(coin))
+				suite.Require().NoError(err)
+				topics = []common.Hash{
+					evmhandlers.SendCroToIbcEvent.ID,
+				}
+				input, _ := evmhandlers.SendToIbcEvent.Inputs.NonIndexed().Pack(
+					sender,
+					"recipient",
+					coin.Amount.BigInt(),
+				)
+				data = input
+			},
+			func() {},
+			fmt.Errorf("contract %s is not authorized to use SendCroToIbc hook", contract),
+		},
 		{
 			"not enough balance, fail",
 			func() {
+				params := suite.app.CronosKeeper.GetParams(suite.ctx)
+				params.CroBridgeContractAddresses = []string{contract.Hex()}
+				err := suite.app.CronosKeeper.SetParams(suite.ctx, params)
+				suite.Require().NoError(err)
+
 				coin := sdk.NewCoin(suite.evmParam.EvmDenom, sdkmath.NewInt(10000000000000))
 				topics = []common.Hash{
 					evmhandlers.SendCroToIbcEvent.ID,
@@ -377,8 +430,13 @@ func (suite *KeeperTestSuite) TestSendCroToIbcHandler() {
 		{
 			"success send cro to ibc",
 			func() {
+				params := suite.app.CronosKeeper.GetParams(suite.ctx)
+				params.CroBridgeContractAddresses = []string{contract.Hex()}
+				err := suite.app.CronosKeeper.SetParams(suite.ctx, params)
+				suite.Require().NoError(err)
+
 				coin := sdk.NewCoin(suite.evmParam.EvmDenom, sdkmath.NewInt(1230000000500))
-				err := suite.MintCoins(contract.Bytes(), sdk.NewCoins(coin))
+				err = suite.MintCoins(contract.Bytes(), sdk.NewCoins(coin))
 				suite.Require().NoError(err)
 
 				balance := suite.app.BankKeeper.GetBalance(suite.ctx, contract.Bytes(), suite.evmParam.EvmDenom)

x/cronos/simulation/genesis.go

@@ -77,11 +77,11 @@ func RandomizedGenState(simState *module.SimulationState) {
 	)
 
 	simState.AppParams.GetOrGenerate(
-		maxCallbackGasKey, &ibcTimeout, simState.Rand,
-		func(r *rand.Rand) { maxCallbackGas = GenIbcTimeout(r) },
+		maxCallbackGasKey, &maxCallbackGas, simState.Rand,
+		func(r *rand.Rand) { maxCallbackGas = GenMaxCallbackGas(r) },
 	)
 
-	params := types.NewParams(ibcCroDenom, ibcTimeout, cronosAdmin, enableAutoDeployment, maxCallbackGas)
+	params := types.NewParams(ibcCroDenom, ibcTimeout, cronosAdmin, enableAutoDeployment, maxCallbackGas, []string{})
 	cronosGenesis := &types.GenesisState{
 		Params:            params,
 		ExternalContracts: nil,