c4-bot-2 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L401-L407 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L425-L432

Vulnerability details

Impact

The CollateralTracker.previewDeposit function is used by the CollateralTracker.deposit function and the amount of shares to mint is calculated as follows:

        unchecked {
            shares = Math.mulDiv( //@audit-info - commissions are only paid when a new position is minted
                assets * (DECIMALS - COMMISSION_FEE), //@audit-info - COMMISSION_FEE -> single payment of the commissionRate on the FINAL (post mev-tax) assets paid
                totalSupply,
                totalAssets() * DECIMALS
            );

But the issue here is for the first depositor the totalSupply == 0 (no shares are minted yet). As a result the calculated number of shares for the provided asset amount will be 0. The CollateralTracker.deposit function will transfer the assets amount to the respective PanopticPool but the shares minted to the receiver will be 0. As a result the msg.sender or the receiver has lost his funds.

If another user mints shares by calling the CollateralTracker.mint function as the first depositor by supplying the required number of assets, then he can withdraw the funds of the previous depositor (who lost the funds) as well by redeeming his shares thus profiting as a result.

Proof of Concept

        unchecked {
            shares = Math.mulDiv(
                assets * (DECIMALS - COMMISSION_FEE),
                totalSupply,
                totalAssets() * DECIMALS
            );
        }

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L401-L407

            s_underlyingToken,
            msg.sender,
            address(s_panopticPool),
            assets
        );

        // mint collateral shares of the Panoptic Pool funds (this ERC20 token)
        _mint(receiver, shares);

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L425-L432

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to assign assets amount to the shares variable in the CollateralTracker.previewDeposit function, such that the first depositor calling the deposit function will not lose his funds.

Assessed type

ERC4626

c4-judge commented 2 months ago

Picodes marked the issue as primary issue

dyedm1 commented 2 months ago

This is not true. totalSupply is initialized to 10**6 and assets is initialized to 1.