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

Lack of Arbitrum Sequencer Uptime Checks in CollateralTracker Contract #546

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

Comments

@c4-bot-8
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

The CollateralTracker smart contract lacks mechanisms to check the uptime of the Arbitrum sequencer. This oversight can lead to the processing of transactions based on outdated or incorrect market data during periods of sequencer downtime, potentially causing financial inaccuracies and security risks.

Functions like takeCommissionAddData and exercise, which rely heavily on current market data for calculating fees or exercising options, may operate on outdated information, resulting in improper fee deductions or option executions.

Proof of Concept

Loc

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

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

function takeCommissionAddData(
        address optionOwner,
        int128 longAmount,
        int128 shortAmount,
        int128 swappedAmount
    ) external onlyPanopticPool returns (int256 utilization) {
        unchecked {
            // current available assets belonging to PLPs (updated after settlement) excluding any premium paid
            int256 updatedAssets = int256(uint256(s_poolAssets)) - swappedAmount;

            // constrict premium to only assets not belonging to PLPs (i.e premium paid by sellers or collected from the pool earlier)
            int256 tokenToPay = _getExchangedAmount(longAmount, shortAmount, swappedAmount);

            // compute tokens to be paid due to swap
            // mint or burn tokens due to minting in-the-money
            if (tokenToPay > 0) {
                // if user must pay tokens, burn them from user balance
                uint256 sharesToBurn = Math.mulDivRoundingUp(
                    uint256(tokenToPay),
                    totalSupply,
                    totalAssets()
                );
                _burn(optionOwner, sharesToBurn);
            } else if (tokenToPay < 0) {
                // if user must receive tokens, mint them
                uint256 sharesToMint = convertToShares(uint256(-tokenToPay));
                _mint(optionOwner, sharesToMint);
            }

            // update stored asset balances with net moved amounts
            // the inflow or outflow of pool assets is defined by the swappedAmount: it includes both the ITM swap amounts and the short/long amounts used to create the position
            // however, any intrinsic value is paid for by the users, so we only add the portion that comes from PLPs: the short/long amounts
            // premia is not included in the balance since it is the property of options buyers and sellers, not PLPs
            s_poolAssets = uint128(uint256(updatedAssets));
            s_inAMM = uint128(uint256(int256(uint256(s_inAMM)) + (shortAmount - longAmount)));

            utilization = _poolUtilization();
        }
    }
function exercise(
        address optionOwner,
        int128 longAmount,
        int128 shortAmount,
        int128 swappedAmount,
        int128 realizedPremium
    ) external onlyPanopticPool returns (int128) {
        unchecked {
            // current available assets belonging to PLPs (updated after settlement) excluding any premium paid
            int256 updatedAssets = int256(uint256(s_poolAssets)) - swappedAmount;

            // add premium to be paid/collected on position close
            int256 tokenToPay = -realizedPremium;

            // if burning ITM and swap occurred, compute tokens to be paid through exercise and add swap fees
            int256 intrinsicValue = swappedAmount - (longAmount - shortAmount);

            if ((intrinsicValue != 0) && ((shortAmount != 0) || (longAmount != 0))) {
                // intrinsic value is the amount that need to be exchanged due to burning in-the-money

                // add the intrinsic value to the tokenToPay
                tokenToPay += intrinsicValue;
            }

            if (tokenToPay > 0) {
                // if user must pay tokens, burn them from user balance (revert if balance too small)
                uint256 sharesToBurn = Math.mulDivRoundingUp(
                    uint256(tokenToPay),
                    totalSupply,
                    totalAssets()
                );
                _burn(optionOwner, sharesToBurn);
            } else if (tokenToPay < 0) {
                // if user must receive tokens, mint them
                uint256 sharesToMint = convertToShares(uint256(-tokenToPay));
                _mint(optionOwner, sharesToMint);
            }

            // update stored asset balances with net moved amounts
            // any intrinsic value is paid for by the users, so we do not add it to s_inAMM
            // premia is not included in the balance since it is the property of options buyers and sellers, not PLPs
            s_poolAssets = uint128(uint256(updatedAssets + realizedPremium));
            s_inAMM = uint128(uint256(int256(uint256(s_inAMM)) - (shortAmount - longAmount)));

            return (int128(tokenToPay));
        }
    }

Tools Used

Manual

Recommended Mitigation Steps

Implement checks using tools such as Chainlink's Sequencer Uptime Feeds to determine the operational status of the Arbitrum sequencer. Pause or modify the execution of sensitive transactions during detected downtimes.

Assessed type

Invalid Validation

@c4-bot-8 c4-bot-8 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-5 added a commit that referenced this issue Apr 22, 2024
@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-a 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

3 participants