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

2 stars 2 forks source link

Lack of slippage control in CollateralTracker #543

Closed c4-bot-9 closed 2 months ago

c4-bot-9 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L591 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L531 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L417

Vulnerability details

Impact

Lack of slippage control can potentially result in user receiving less than expected shares and assets

Proof of Concept

CollatralTracker is an ERC4626 vault where token liquidity from passive Panoptic Liquidity Providers (PLPs) and collateral for option positions are deposited.

Users utilize the deposit() function to mint shares. Similarly, shares can be redeemed through the withdraw() and redeem() functions. However, these functions lack protection from slippage control, exposing users to the risk of receiving significantly fewer shares or assets. Attackers can exploit this vulnerability by front-running and creating unfavorable scenarios for users.

    function deposit(uint256 assets, address receiver) external returns (uint256 shares) {
        if (assets > type(uint104).max) revert Errors.DepositTooLarge();

        shares = previewDeposit(assets);

        // transfer assets (underlying token funds) from the user/the LP to the PanopticPool
        // in return for the shares to be minted
        SafeTransferLib.safeTransferFrom(
            s_underlyingToken,
            msg.sender,
            address(s_panopticPool),
            assets
        );

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

        // update tracked asset balance
        unchecked {
            s_poolAssets += uint128(assets);
        }

        emit Deposit(msg.sender, receiver, assets, shares);
    }
    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) external returns (uint256 shares) {
        if (assets > maxWithdraw(owner)) revert Errors.ExceedsMaximumRedemption();

        shares = previewWithdraw(assets);

        // check/update allowance for approved withdraw
        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares;
        }

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

        // update tracked asset balance
        unchecked {
            s_poolAssets -= uint128(assets);
        }

        // transfer assets (underlying token funds) from the PanopticPool to the LP
        SafeTransferLib.safeTransferFrom(
            s_underlyingToken,
            address(s_panopticPool),
            receiver,
            assets
        );

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        return shares;
    }
    function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) external returns (uint256 assets) {
        if (shares > maxRedeem(owner)) revert Errors.ExceedsMaximumRedemption();

        // check/update allowance for approved redeem
        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares;
        }

        assets = previewRedeem(shares);

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

        // update tracked asset balance
        unchecked {
            s_poolAssets -= uint128(assets);
        }

        // transfer assets (underlying token funds) from the PanopticPool to the LP
        SafeTransferLib.safeTransferFrom(
            s_underlyingToken,
            address(s_panopticPool),
            receiver,
            assets
        );

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        return assets;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add slippage protection to all functions above.

Assessed type

MEV

c4-judge commented 2 months ago

Picodes marked the issue as duplicate of #365

c4-judge commented 2 months ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 2 months ago

Picodes marked the issue as grade-c