c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/LiquidityPool.sol#L145-L157

Vulnerability details

Impact

Circumvention of the maximum minting restriction, since all a receiver need to do is not specify the whole amount of tokens in one attempt and then claim more than the maximum amount

Proof of Concept

Take a look at LiquidityPool.sol#L145-L157

 /// @notice Collect shares for deposited assets after Centrifuge epoch execution.
    ///         maxMint is the max amount of shares that can be collected.
    function mint(uint256 shares, address receiver) public returns (uint256 assets) {
        assets = investmentManager.processMint(receiver, shares);
        emit Deposit(address(this), receiver, assets, shares);
    }

    /// @notice Maximum amount of shares that can be claimed by the receiver after the epoch has been executed on the Centrifuge side.
    function maxMint(address receiver) external view returns (uint256 maxShares) {
        maxShares = investmentManager.maxMint(receiver, address(this));
    }

As seen, the below is queried

    /// @notice Processes user's currency deposit / investment after the epoch has been executed on Centrifuge.
    ///         In case user's invest order was fulfilled on Centrifuge during epoch execution MaxDeposit and MaxMint are increased
    ///         and trancheTokens can be transferred to user's wallet on calling processDeposit or processMint.
    ///         Note: The currency amount required to fulfil the invest order is already locked in escrow upon calling requestDeposit.
    ///         Note: The tranche tokens are already minted on collectInvest and are deposited to the escrow account until the users calls mint, or deposit.
    /// @dev    currencyAmount return value is type of uint256 to be compliant with EIP4626 LiquidityPool interface
    /// @return currencyAmount the amount of liquidityPool assets invested and locked in escrow in order
    ///         for the amount of tranche tokens received after successful investment into the pool.
    function processMint(address user, uint256 trancheTokenAmount) public auth returns (uint256 currencyAmount) {
        address liquidityPool = msg.sender;
        uint128 _trancheTokenAmount = _toUint128(trancheTokenAmount);
        require(
        //@audit
            (_trancheTokenAmount <= orderbook[user][liquidityPool].maxMint && _trancheTokenAmount != 0),
            "InvestmentManager/amount-exceeds-mint-limits"
        );

        uint256 depositPrice = calculateDepositPrice(user, liquidityPool);
        require(depositPrice != 0, "LiquidityPool/deposit-token-price-0");

        uint128 _currencyAmount = _calculateCurrencyAmount(_trancheTokenAmount, liquidityPool, depositPrice);
        _deposit(_trancheTokenAmount, _currencyAmount, liquidityPool, user);
        currencyAmount = uint256(_currencyAmount);
    }

The issue as seen is that only the particular instance of minting is checked, and no checks are applied to see if users has previously minted

Tool used

Manual Review

Recommended Mitigation Steps

If users have minted before, the amounts they've minted should be added to the current iteration to see if they are still below their respective maxMint ratio

Assessed type

Token-Transfer

c4-pre-sort commented 1 year ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #118

c4-judge commented 1 year ago

gzeon-c4 marked the issue as not a duplicate