code-423n4 / 2024-04-panoptic-findings

7 stars 3 forks source link

difference in calculation of values in `previewDeposit` and `previewMint` will return different vaules #491

Closed c4-bot-6 closed 4 months ago

c4-bot-6 commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The Protocol implements two different formula for calculation of assets and share values.

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

    function previewDeposit(uint256 assets) public view returns (uint256 shares) {
        // compute the MEV tax, which is equal to a single payment of the commissionRate on the FINAL (post mev-tax) assets paid
        unchecked {
            shares = Math.mulDiv(
                assets * (DECIMALS - COMMISSION_FEE),
                totalSupply,
                totalAssets() * DECIMALS
            );
        }
    }

Proof of Concept

for simplicity i will ignore the decimal portion here suppose we have following values:

DECIMALS = 10000;
COMMISSION_FEE = 10;
totalSupply = 12;
totalAssets = 20;

PreviewDeposit(assets = 864) => returns shares =  517;

PreviewMint(shares= 517) => returns assets = 863;

The user lose 1 assets due to difference in the formula

Tools Used

Manual Review

Recommended Mitigation Steps

Try to implement same formula for both cases.

Assessed type

ERC4626

c4-judge commented 4 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof

Picodes commented 4 months ago

I can't consider a 1 wei diff due to rounding issues a Medium severity finding