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

2 stars 2 forks source link

convertToAssets can return zero when totalAsset * shares is lesser than TotalSupply #572

Closed c4-bot-1 closed 2 months ago

c4-bot-1 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L386 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L507

Vulnerability details

Impact

Users that redeemShares will get 0 tokens while having all their share amount removed if their share amount is lesser than total supply when multiplied with total assets.

POC

Preview redeem

    function previewRedeem(uint256 shares) public view returns (uint256 assets) {
        return convertToAssets(shares);
    }

convertToAssets

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

redeem function when shares is given lower than totalsupply/totalAssets


 function redeem(
        uint256 shares,
        address receiver,
        address owner
    )
{
    .
        assets = previewRedeem(shares);

        // burn collateral shares of the Panoptic Pool funds (this ERC20 token)
        _burn(owner, shares);
    .

Here we see that previewRedeem can return zero but owner's shares are still burned.

Recommended mitigation steps

Add zero value checks against return value of previewRedeem

Assessed type

Math

c4-judge commented 2 months ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 2 months ago

There is nothing more here than the loss rounding down usually lead to