Centrifuge - MaslarovK's results

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L383 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L396

Vulnerability details

Impact

Other protocols might wrongly assume that the functions handle rounding as per ERC4626 expectation. Thus, it might cause some intergration problem in the future that can lead to wide range of issues for both parties.

Proof of Concept

The current implementation of convertToShares function will round down the number of shares returned due to how solidity handles Integer Division. ERC4626 expects the returned value of convertToShares to be rounded down. Thus, this function behaves as expected.

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L325

function convertToShares(uint256 _assets, address liquidityPool) public view auth returns (uint256 shares) {
    (uint8 currencyDecimals, uint8 trancheTokenDecimals) = _getPoolDecimals(liquidityPool);
    uint128 assets = _toUint128(_assets);

    shares = assets.mulDiv(
        10 ** (PRICE_DECIMALS + trancheTokenDecimals - currencyDecimals),
        LiquidityPoolLike(liquidityPool).latestPrice(),
        MathLib.Rounding.Down
    );
}

ERC 4626 expects the result returned from previewWithdraw function to be rounded up. However, within the previewWithdraw function, it calls the convertToShares function. Recall earlier that the convertToShares function returned a rounded down value, thus previewWithdraw will return a rounded down value instead of round up value. Thus, this function does not behave as expected.

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L396

function previewWithdraw(address user, address liquidityPool, uint256 _currencyAmount)
    public
    view
    returns (uint256 trancheTokenAmount)
 {
    uint128 currencyAmount = _toUint128(_currencyAmount);
    uint256 redeemPrice = calculateRedeemPrice(user, liquidityPool);
    if (redeemPrice == 0) return 0;

    trancheTokenAmount = uint256(_calculateTrancheTokenAmount(currencyAmount, liquidityPool,redeemPrice));
}

previewWithdraw and previewMint functions rely on _calculateTrancheTokenAmount and _calculateCurrencyAmount functions. A rounded down value is returned from these functions, which will cause previewWithdraw and previewMint functions to not behave as expected.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure that the rounding of vault's functions behave as expected. Following are the expected rounding direction for each vault function:

previewMint(uint256 shares) - Round Up ⬆

previewWithdraw(uint256 assets) - Round Up ⬆

previewRedeem(uint256 shares) - Round Down ⬇

previewDeposit(uint256 assets) - Round Down ⬇

convertToAssets(uint256 shares) - Round Down ⬇

convertToShares(uint256 assets) - Round Down ⬇

previewMint returns the amount of assets that would be deposited to mint specific amount of shares. Thus, the amount of assets must be rounded up, so that the vault won't be shortchanged.

previewWithdraw returns the amount of shares that would be burned to withdraw specific amount of asset. Thus, the amount of shares must to be rounded up, so that the vault won't be shortchanged.

Assessed type

ERC4626