Calculation of CollateralTracker.maxMint function is faulty

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L444-L448
https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L379-L381
https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L453-L468
https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L477-L500

Vulnerability details

Impact

The following maxMint function returns maxShares that is (convertToShares(type(uint104).max) * DECIMALS) / (DECIMALS + COMMISSION_FEE), which can be rewritten as (type(uint104).max * totalSupply / totalAssets()) * DECIMALS / (DECIMALS + COMMISSION_FEE).

https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L444-L448

    function maxMint(address) external view returns (uint256 maxShares) {
        unchecked {
            return (convertToShares(type(uint104).max) * DECIMALS) / (DECIMALS + COMMISSION_FEE);
        }
    }

https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L379-L381

    function convertToShares(uint256 assets) public view returns (uint256 shares) {
        return Math.mulDiv(assets, totalSupply, totalAssets());
    }

Calling the following previewMint function with the maxShares returned by the maxMint function as the shares input would return an assets that is ((type(uint104).max * totalSupply / totalAssets()) * DECIMALS / (DECIMALS + COMMISSION_FEE)) * DECIMALS * totalAssets() / (totalSupply * (DECIMALS - COMMISSION_FEE)), which can be simplified to type(uint104).max * (DECIMALS**2) / (DECIMALS**2 - COMMISSION_FEE**2). Since (DECIMALS**2) / (DECIMALS**2 - COMMISSION_FEE**2) is bigger than 1 for a reasonable COMMISSION_FEE, type(uint104).max * (DECIMALS**2) / (DECIMALS**2 - COMMISSION_FEE**2) would be larger than type(uint104).max. This means that such assets, which corresponds to the maxShares returned by the maxMint function, is larger than type(uint104).max that is the deposit limit of the underlying asset; this shows that the calculation of the maxMint function is faulty.

https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L453-L468

    function previewMint(uint256 shares) public view returns (uint256 assets) {
        ...
        unchecked {
            assets = Math.mulDivRoundingUp(
                shares * DECIMALS,
                totalAssets(),
                totalSupply * (DECIMALS - COMMISSION_FEE)
            );
        }
    }

Because of the faulty calculation of the maxMint function, calling the following mint function with the maxShares returned by the maxMint function as the shares input will revert with the DepositTooLarge error.

https://github.com/code-423n4/2024-04-panoptic/blob/58dda1b3b74e48f4d924731ec5da14096043fde0/contracts/CollateralTracker.sol#L477-L500

    function mint(uint256 shares, address receiver) external returns (uint256 assets) {
        assets = previewMint(shares);

        if (assets > type(uint104).max) revert Errors.DepositTooLarge();

        ...
    }

According to https://eips.ethereum.org/EIPS/eip-4626, the EIP-4626 standard requires the maxMint function to return Maximum amount of shares that can be minted from the Vault for the ``receiver``, through a ``mint`` call and MUST return the maximum amount of shares ``mint`` would allow to be deposited to ``receiver`` and not cause a revert. However, calling the mint function with the maxShares returned by the maxMint function as the shares input reverts while such operation should not revert. As a result, the faulty calculation of the maxMint function violates the EIP-4626 standard.

Proof of Concept

Please add the following test in test\foundry\core\CollateralTracker.t.sol. This test will pass to demonstrate the described scenario.

    function test_maxMintFaultyCalculation(uint256 x) public {
        _initWorld(x);

        vm.startPrank(Bob);

        _grantTokens(Bob);
        IERC20Partial(token0).approve(address(collateralToken0), type(uint256).max);

        uint256 maxShares = collateralToken0.maxMint(Bob);

        // calling mint function reverts because maxShares returned by maxMint function corresponds to assets that is more than type(uint104).max
        vm.expectRevert(Errors.DepositTooLarge.selector);
        uint256 returnedAssets0 = collateralToken0.mint(maxShares, Bob);

        vm.stopPrank();
    }

Tools Used

Manual Review

Recommended Mitigation Steps

The maxMint function can be updated to the following code.

    function maxMint(address) external view returns (uint256 maxShares) {
        unchecked {
            maxShares = Math.mulDiv(
                type(uint104).max * (DECIMALS - COMMISSION_FEE),
                totalSupply,
                totalAssets() * DECIMALS
            );
        }
    }

Assessed type

ERC4626

[c4-judge]

Picodes marked the issue as primary issue

[c4-judge]

Picodes marked the issue as duplicate of #370

[c4-judge]

Picodes marked the issue as duplicate of #553

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)