Centrifuge - maanas's results

Lines of code

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

Vulnerability details

Impact

Due to the onlyCentrifugeChainOrigin() modifier, the execute() function in Axelar router will always revert as the msg.sender will not be the Axelar Gateway. https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/gateway/routers/axelar/Router.sol#L43

    modifier onlyCentrifugeChainOrigin(string calldata sourceChain, string calldata sourceAddress) {
        require(msg.sender == address(axelarGateway), "AxelarRouter/invalid-origin");
       ..... more code
    }

The check is a wrong check as the Axelar Gateway only approves the call and doesn't actually make the call. Hence all cross-chain messages using Axelar router will fail.

Proof of Concept

From the documentation of Axelar: https://docs.axelar.dev/learn/network/flow

After a relayer sends it out, a Gateway on the destination chain receives the approved message, where its payload is marked as approved by Axelar. The Gateway stores the contract call approval along with a hash of the payload, representing the legitimacy of this cross-chain message. The approved payload can now be executed by anyone at any time.

In the following transaction containing Axelar callContract, it can be seen that at the executed step, the call to the destination contract is made by Axelar Relayer and not Axelar Gateway https://axelarscan.io/gmp/0x7baba0fb9de79efc8eb8f343dd0e3b0b9d253f6a304a7f96f898c0aee8d21fff:285

Tools Used

Documentation

Recommended Mitigation Steps

Remove the msg.sender check. The validity of the incoming call is established by calling the validateContractCall() function of the Axelar gateway as implemented inside the execute() function of the router. https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/gateway/routers/axelar/Router.sol#L73-L86

    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);
    }

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L551-L558

Vulnerability details

Impact

When depositing, the price calculated for the tranche token is rounded down leading to excess tranche tokens being transferred to the investor.

The price of the tranche token is calculated using the calculateDepositPrice() function. This function in turn calls the _calculatePrice() function which always rounds down after division.

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L551-L558

    function calculateDepositPrice(address user, address liquidityPool) public view returns (uint256 depositPrice) {
        LPValues storage lpValues = orderbook[user][liquidityPool];
        if (lpValues.maxMint == 0) {
            return 0;
        }


        depositPrice = _calculatePrice(lpValues.maxDeposit, lpValues.maxMint, liquidityPool);
    }

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L569-L582

    function _calculatePrice(uint128 currencyAmount, uint128 trancheTokenAmount, address liquidityPool)
        public
        view
        returns (uint256 depositPrice)
    {
        (uint8 currencyDecimals, uint8 trancheTokenDecimals) = _getPoolDecimals(liquidityPool);
        uint256 currencyAmountInPriceDecimals = _toPriceDecimals(currencyAmount, currencyDecimals, liquidityPool);
        uint256 trancheTokenAmountInPriceDecimals =
            _toPriceDecimals(trancheTokenAmount, trancheTokenDecimals, liquidityPool);


        depositPrice = currencyAmountInPriceDecimals.mulDiv(
            10 ** PRICE_DECIMALS, trancheTokenAmountInPriceDecimals, MathLib.Rounding.Down
        );
    }

A lowered price for the tranche token would result in more tranche tokens to be transferred to the investor than what was originally minted towards this deposit. Based on the condition of the escrow, this can lead to either this deposit reverting or future deposits reverting.

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L234-L253

    function handleExecutedCollectInvest(
        uint64 poolId,
        bytes16 trancheId,
        address recipient,
        uint128 currency,
        uint128 currencyPayout,
        uint128 trancheTokensPayout
    ) public onlyGateway {
        require(currencyPayout != 0, "InvestmentManager/zero-invest");
        address _currency = poolManager.currencyIdToAddress(currency);
        address liquidityPool = poolManager.getLiquidityPool(poolId, trancheId, _currency);
        require(liquidityPool != address(0), "InvestmentManager/tranche-does-not-exist");


        LPValues storage lpValues = orderbook[recipient][liquidityPool];
        lpValues.maxDeposit = lpValues.maxDeposit + currencyPayout;
        lpValues.maxMint = lpValues.maxMint + trancheTokensPayout;

// @audit: fixed number of tranche tokens are minted for this deposit here
        LiquidityPoolLike(liquidityPool).mint(address(escrow), trancheTokensPayout); // mint to escrow. Recepeint can claim by calling withdraw / redeem
        _updateLiquidityPoolPrice(liquidityPool, currencyPayout, trancheTokensPayout);
    }

Proof of Concept

The following scenario would prove the concept:

1. Two investors invest (requestDeposit) x amount of assets to a liquidity pool.
2. The requestDeposit() orders get executed at the same price and hence both user's are now eligible to deposit x amount of asset's and mint y amount of tranche tokens.
3. First user deposit's x amount of asset and receive > y amount of tranche tokens.
4. When the second user attempts to deposit x amount of asset, it reverts due to the escrow having < y amount of tranche tokens remaining.

The code for the above scenario is attached below. Add this test to test/LiquidityPool.t.sol and run forge test --match-test testDepositFailsDueToRounding

   function testDepositFailsDueToRounding(uint64 poolId, bytes16 trancheId, uint128 currencyId) public {
        vm.assume(currencyId > 0);

        uint8 TRANCHE_TOKEN_DECIMALS = 6;
        uint8 INVESTMENT_CURRENCY_DECIMALS = 9;

        ERC20 currency = _newErc20("Currency", "CR", INVESTMENT_CURRENCY_DECIMALS);
        address lPool_ =
            deployLiquidityPool(poolId, TRANCHE_TOKEN_DECIMALS, "", "", trancheId, currencyId, address(currency));
        LiquidityPool lPool = LiquidityPool(lPool_);
        uint128 _currencyId = poolManager.currencyAddressToId(address(currency));

        // two investor invests the same amount and gets their order fulfilled at the same price

        uint256 investmentAmount = 1000000000;
        uint128 currencyPayout = 1000000000;
        uint128 trancheTokenPayout = 8412560094779513;

        address investor1 = address(1);

        // investor 1 invests
        {
            currency.mint(investor1, investmentAmount);

            homePools.updateMember(poolId, trancheId, investor1, type(uint64).max);
            vm.startPrank(investor1);
            currency.approve(address(investmentManager), investmentAmount);
            lPool.requestDeposit(investmentAmount, investor1);
            vm.stopPrank();

            homePools.isExecutedCollectInvest(
                poolId, trancheId, bytes32(bytes20(investor1)), _currencyId, currencyPayout, trancheTokenPayout
            );

            // assert deposit & mint values adjusted
            assertEq(lPool.maxDeposit(investor1), currencyPayout);
            assertEq(lPool.maxMint(investor1), trancheTokenPayout);
        }

        address investor2 = address(2);
        {
            // investor 2 invests

            currency.mint(investor2, investmentAmount);

            homePools.updateMember(poolId, trancheId, investor2, type(uint64).max);
            vm.startPrank(investor2);
            currency.approve(address(investmentManager), investmentAmount);
            lPool.requestDeposit(investmentAmount, investor2);
            vm.stopPrank();

            homePools.isExecutedCollectInvest(
                poolId, trancheId, bytes32(bytes20(investor2)), _currencyId, currencyPayout, trancheTokenPayout
            );

            // assert deposit & mint values adjusted
            assertEq(lPool.maxDeposit(investor2), currencyPayout);
            assertEq(lPool.maxMint(investor2), trancheTokenPayout);
        }

        // due to rounding issue, investor1 receives more trancheTokenAmount than expected
        {
            // investor1 deposits maximum
            lPool.deposit(currencyPayout, investor1);
            assertGt(lPool.balanceOf(investor1), trancheTokenPayout);
        }

        // since overall trancheTokenPayout + trancheTokenPayout has only been minted, the next investor cannot deposit his maximum
        {
            // investor1 deposits maximum
            vm.expectRevert();
            lPool.deposit(currencyPayout, investor2);
        }
    }

Tools Used

Foundry

Recommended Mitigation Steps

1. Have the price rounded up in case of deposits and the price rounded down in case of redeem.
2. Or have the tokens minted when processDeposit is called.

Assessed type

Math