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

7 stars 3 forks source link

User can avoid force exercise by front-running and mint/burn another position #532

Closed c4-bot-1 closed 4 months ago

c4-bot-1 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1268 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L893 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1367-#L1395

Vulnerability details

Vulnerability details

When calling forceExercise() function, _validateSolvency() is called to check positionIdList of account:

     _validateSolvency(account, positionIdListExercisee, NO_BUFFER);  // <---

It will validate position list of account:

function _validateSolvency(
    address user,
    TokenId[] calldata positionIdList,
    uint256 buffer
) internal view returns (uint256 medianData) {
    // check that the provided positionIdList matches the positions in memory
    _validatePositionList(user, positionIdList, 0);   // <---

This function will revert if hash is incorrect:

function _validatePositionList(
    address account,
    TokenId[] calldata positionIdList,
    uint256 offset
) internal view {
    uint256 pLength;
    uint256 currentHash = s_positionsHash[account];  // <---

    unchecked {
        pLength = positionIdList.length - offset;
    }
    // note that if pLength == 0 even if a user has existing position(s) the below will fail b/c the fingerprints will mismatch
    // Check that position hash (the fingerprint of option positions) matches the one stored for the '_account'
    uint256 fingerprintIncomingList;

    for (uint256 i = 0; i < pLength; ) {
        fingerprintIncomingList = PanopticMath.updatePositionsHash(
            fingerprintIncomingList,
            positionIdList[i],
            ADD
        );
        unchecked {
            ++i;
        }
    }

    // revert if fingerprint for provided '_positionIdList' does not match the one stored for the '_account'
    if (fingerprintIncomingList != currentHash) revert Errors.InputListFail();  // <---

And the hash is updated whenever nwe position of user is updated/removed by calling _updatePositionsHash() function:

function _updatePositionsHash(address account, TokenId tokenId, bool addFlag) internal {
    uint256 newHash = PanopticMath.updatePositionsHash(
        s_positionsHash[account],
        tokenId,
        addFlag
    );
    if ((newHash >> 248) > MAX_POSITIONS) revert Errors.TooManyPositionsOpen();
    s_positionsHash[account] = newHash;   // <---
}

It lead to scenario that account can avoid being forced to execute by minting/burning other option that to make sure numbers of option of the user is is smaller than MAX_POSITIONS

Impact

Option can be avoid to be force executed, user's funds is locked.

Tools Used

Manual review

Recommended Mitigation Steps

Mechanism related to hash of position list should be changed.

Assessed type

Context

c4-judge commented 4 months ago

Picodes marked the issue as duplicate of #352

c4-judge commented 4 months ago

Picodes changed the severity to QA (Quality Assurance)