handle functions cannot be usable.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/gateway/routers/axelar/Router.sol#L73-L86 https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/gateway/routers/axelar/Router.sol#L43-L54

Vulnerability details

Impact

Protocol's important function called by gateaway contract's handle function.

    function handle(bytes calldata message) external onlyIncomingRouter pauseable {
        if (Messages.isAddCurrency(message)) {
            (uint128 currency, address currencyAddress) = Messages.parseAddCurrency(message);
            poolManager.addCurrency(currency, currencyAddress);
        } else if (Messages.isAddPool(message)) {
            (uint64 poolId) = Messages.parseAddPool(message);
            poolManager.addPool(poolId);
        } else if (Messages.isAllowPoolCurrency(message)) {
            (uint64 poolId, uint128 currency) = Messages.parseAllowPoolCurrency(message);
            poolManager.allowPoolCurrency(poolId, currency);
        } else if (Messages.isAddTranche(message)) {
            (
                uint64 poolId,
                bytes16 trancheId,
                string memory tokenName,
                string memory tokenSymbol,
                uint8 decimals,
                uint128 _price
            ) = Messages.parseAddTranche(message);
            poolManager.addTranche(poolId, trancheId, tokenName, tokenSymbol, decimals);
        } else if (Messages.isUpdateMember(message)) {
            (uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) = Messages.parseUpdateMember(message);
            poolManager.updateMember(poolId, trancheId, user, validUntil);
        } else if (Messages.isUpdateTrancheTokenPrice(message)) {
            (uint64 poolId, bytes16 trancheId, uint128 currencyId, uint128 price) =
                Messages.parseUpdateTrancheTokenPrice(message);
            investmentManager.updateTrancheTokenPrice(poolId, trancheId, currencyId, price);
        } else if (Messages.isTransfer(message)) {
            (uint128 currency, address recipient, uint128 amount) = Messages.parseIncomingTransfer(message);
            poolManager.handleTransfer(currency, recipient, amount);
        } else if (Messages.isTransferTrancheTokens(message)) {
            (uint64 poolId, bytes16 trancheId, address destinationAddress, uint128 amount) =
                Messages.parseTransferTrancheTokens20(message);
            poolManager.handleTransferTrancheTokens(poolId, trancheId, destinationAddress, amount);
        } else if (Messages.isExecutedDecreaseInvestOrder(message)) {
            (uint64 poolId, bytes16 trancheId, address investor, uint128 currency, uint128 currencyPayout) =
                Messages.parseExecutedDecreaseInvestOrder(message);
            investmentManager.handleExecutedDecreaseInvestOrder(poolId, trancheId, investor, currency, currencyPayout);
        } else if (Messages.isExecutedDecreaseRedeemOrder(message)) {
            (uint64 poolId, bytes16 trancheId, address investor, uint128 currency, uint128 trancheTokensPayout) =
                Messages.parseExecutedDecreaseRedeemOrder(message);
            investmentManager.handleExecutedDecreaseRedeemOrder(
                poolId, trancheId, investor, currency, trancheTokensPayout
            );
        } else if (Messages.isExecutedCollectInvest(message)) {
            (
                uint64 poolId,
                bytes16 trancheId,
                address investor,
                uint128 currency,
                uint128 currencyPayout,
                uint128 trancheTokensPayout
            ) = Messages.parseExecutedCollectInvest(message);
            investmentManager.handleExecutedCollectInvest(
                poolId, trancheId, investor, currency, currencyPayout, trancheTokensPayout
            );
        } else if (Messages.isExecutedCollectRedeem(message)) {
            (
                uint64 poolId,
                bytes16 trancheId,
                address investor,
                uint128 currency,
                uint128 currencyPayout,
                uint128 trancheTokensPayout
            ) = Messages.parseExecutedCollectRedeem(message);
            investmentManager.handleExecutedCollectRedeem(
                poolId, trancheId, investor, currency, currencyPayout, trancheTokensPayout
            );
        } else if (Messages.isScheduleUpgrade(message)) {
            address target = Messages.parseScheduleUpgrade(message);
            root.scheduleRely(target);
        } else if (Messages.isCancelUpgrade(message)) {
            address target = Messages.parseCancelUpgrade(message);
            root.cancelRely(target);
        } else if (Messages.isUpdateTrancheTokenMetadata(message)) {
            (uint64 poolId, bytes16 trancheId, string memory tokenName, string memory tokenSymbol) =
                Messages.parseUpdateTrancheTokenMetadata(message);
            poolManager.updateTrancheTokenMetadata(poolId, trancheId, tokenName, tokenSymbol);
        } else {
            revert("Gateway/invalid-message");
        }
    }

And This function should be called by router. router has execute function to call handle

    function execute(
        bytes32 commandId,
        string calldata sourceChain,
        string calldata sourceAddress,
        bytes calldata payload
    ) public onlyCentrifugeChainOrigin(sourceChain, sourceAddress) {
        bytes32 payloadHash = keccak256(payload);
        require(
            axelarGateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash),
            "Router/not-approved-by-gateway"
        );

        gateway.handle(payload);
    }

This execute function use onlyCentrifugeChainOrigin modifier.

    modifier onlyCentrifugeChainOrigin(string calldata sourceChain, string calldata sourceAddress) {
        require(msg.sender == address(axelarGateway), "AxelarRouter/invalid-origin");
        require(
            keccak256(bytes(axelarCentrifugeChainId)) == keccak256(bytes(sourceChain)),
            "AxelarRouter/invalid-source-chain"
        );
        require(
            keccak256(bytes(axelarCentrifugeChainAddress)) == keccak256(bytes(sourceAddress)),
            "AxelarRouter/invalid-source-address"
        );
        _;
    }

From this modifier it can be seen that only axelargateaway can call this function but the main problem is axelargateaway has not functionality to make external calls it just verify interchain messages. Other addresses should make calls verified calls.

Proof of Concept

From axelargateaway contract https://github.com/axelarnetwork/axelar-cgp-solidity/blob/main/contracts/AxelarGateway.sol it can be seen that, execute function can make verification of call paramaters(address,data) but don't call directly.

    function execute(bytes calldata input) external override {
        (bytes memory data, bytes memory proof) = abi.decode(input, (bytes, bytes));

        bytes32 messageHash = ECDSA.toEthSignedMessageHash(keccak256(data));

        // returns true for current operators
        // slither-disable-next-line reentrancy-no-eth
        bool allowOperatorshipTransfer = IAxelarAuth(authModule).validateProof(messageHash, proof);

        uint256 chainId;
        bytes32[] memory commandIds;
        string[] memory commands;
        bytes[] memory params;

        (chainId, commandIds, commands, params) = abi.decode(data, (uint256, bytes32[], string[], bytes[]));

        if (chainId != block.chainid) revert InvalidChainId();

        uint256 commandsLength = commandIds.length;

        if (commandsLength != commands.length || commandsLength != params.length) revert InvalidCommands();

        for (uint256 i; i < commandsLength; ++i) {
            bytes32 commandId = commandIds[i];

            // Ignore if duplicate commandId received
            if (isCommandExecuted(commandId)) continue;

            bytes4 commandSelector;
            bytes32 commandHash = keccak256(abi.encodePacked(commands[i]));

            if (commandHash == SELECTOR_DEPLOY_TOKEN) {
                commandSelector = AxelarGateway.deployToken.selector;
            } else if (commandHash == SELECTOR_MINT_TOKEN) {
                commandSelector = AxelarGateway.mintToken.selector;
            } else if (commandHash == SELECTOR_APPROVE_CONTRACT_CALL) {
                commandSelector = AxelarGateway.approveContractCall.selector;
            } else if (commandHash == SELECTOR_APPROVE_CONTRACT_CALL_WITH_MINT) {
                commandSelector = AxelarGateway.approveContractCallWithMint.selector;
            } else if (commandHash == SELECTOR_BURN_TOKEN) {
                commandSelector = AxelarGateway.burnToken.selector;
            } else if (commandHash == SELECTOR_TRANSFER_OPERATORSHIP) {
                if (!allowOperatorshipTransfer) continue;

                allowOperatorshipTransfer = false;
                commandSelector = AxelarGateway.transferOperatorship.selector;
            } else {
                // Ignore unknown commands
                continue;
            }

            // Prevent a re-entrancy from executing this command before it can be marked as successful.
            _setCommandExecuted(commandId, true);

            // slither-disable-next-line calls-loop,reentrancy-no-eth
            (bool success, ) = address(this).call(abi.encodeWithSelector(commandSelector, params[i], commandId));

            // slither-disable-next-line reentrancy-events
            if (success) emit Executed(commandId);
            else _setCommandExecuted(commandId, false);
        }
    }

If you look at other functions, it can be seen that there is no way for make call for interchain messages too. So in short it can be said that router will never be executed so handle and protocol important functions too. Also from this link :https://docs.axelar.dev/learn/network/flow#submitting-a-message-to-the-destination-chain It can be seen that relayer services will call this approved messages in destination chain but not directly axelargateaway do.

Tools Used

Recommended Mitigation Steps

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #212

[c4-pre-sort]

raymondfam marked the issue as duplicate of #537

[c4-judge]

gzeon-c4 changed the severity to 2 (Med Risk)

[c4-judge]

gzeon-c4 marked the issue as satisfactory