Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A commission_fee is charged when depositing assets to CollateralTracker via deposit() #416

Closed
c4-bot-6 opened this issue Apr 22, 2024 · 2 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@c4-bot-6
Copy link
Contributor

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#L1096-L1126

Vulnerability details

Impact

Each time user go for collateral deposit he subject to pay a COMMISSION_FEE. Its get more difficult for user when he try to save himself from liquidation and add collateral, some of its fund moved as a commision fee

Proof of Concept

Here Deposit function as follows

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

        shares = previewDeposit(assets); 
.....
.....

        emit Deposit(msg.sender, receiver, assets, shares);
    }

which calls previewDeposit() which is implemented as follows

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

But point is if you goes to Developer code comment on COMMISSION_FEE which says following

    /// @notice when creating an option, collect a commission for the Panoptic LPs.
    /// In Panoptic, options never expire, commissions are only paid when a new position is minted.
    /// We believe that this will eliminate the impact of the commission fee on the user's decision-making process when closing a position.
    uint256 immutable COMMISSION_FEE;  

And docmentation also clear that COMMISSION_FEE only applied when new option created(minted), not when asset deposited. below are links

https://panoptic.xyz/docs/panoptic-protocol/specs#commission-fee
https://panoptic.xyz/docs/panoptic-protocol/commission

This also implemented in _getExchangedAmount() which returns exchangedAmount The amount of funds to be exchanged for minting an option (includes commission, swapFee, and intrinsic value).

    function _getExchangedAmount(
        int128 longAmount,
        int128 shortAmount,
        int128 swappedAmount
    ) internal view returns (int256 exchangedAmount) {
...
...

            //compute total commission amount = commission rate + spread fee
            exchangedAmount += int256(
                Math.unsafeDivRoundingUp(
                    uint256(uint128(shortAmount + longAmount)) * COMMISSION_FEE,
                    DECIMALS
                )
            );
        }
    }

So problem here is that protocol charging fee on each deposit, although there is no mention of it.

Tools Used

Manual Review

Recommended Mitigation Steps

As per code comments & Documentation, Panoptic should only charge COMMISSION_FEE when new position minted not in every collateral deposit.

Assessed type

Other

@c4-bot-6 c4-bot-6 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Apr 22, 2024
c4-bot-7 added a commit that referenced this issue Apr 22, 2024
@Picodes
Copy link

Picodes commented Apr 25, 2024

It seems to be working as designed

@c4-judge
Copy link
Contributor

Picodes changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

4 participants