code-423n4 / 2024-02-tapioca-findings

1 stars 1 forks source link

If a user has not removed their funds from `tOLP`, by the time `Singularity` is removed, funds will be lost forever. #50

Closed c4-bot-1 closed 2 months ago

c4-bot-1 commented 3 months ago

Lines of code

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/options/TapiocaOptionLiquidityProvision.sol#L232

Vulnerability details

Impact

Loss of funds

Proof of Concept

Within the tOLP contract, in certain scenarios, Singularities can be removed. The problem is that once a Singularity is removed, if a user by that time has not withdrawn their assets, they're forever lost.

   function unlock(uint256 _tokenId, IERC20 _singularity, address _to) external {
        if (!_exists(_tokenId)) revert PositionExpired();

        LockPosition memory lockPosition = lockPositions[_tokenId];
        SingularityPool memory sgl = activeSingularities[_singularity];

        // If the singularity is in rescue, the lock can be unlocked at any time
        if (!sgl.rescue) {
            // If not, the lock must be expired
            if (block.timestamp < lockPosition.lockTime + lockPosition.lockDuration) revert LockNotExpired();
        }
        if (sgl.sglAssetID != lockPosition.sglAssetID) {
            revert InvalidSingularity();
        }

        if (!_isApprovedOrOwner(msg.sender, _tokenId)) revert NotAuthorized();

        _burn(_tokenId);
        delete lockPositions[_tokenId];

        // Transfer the YieldBox position back to the owner
        yieldBox.transfer(address(this), _to, lockPosition.sglAssetID, lockPosition.ybShares);
        activeSingularities[_singularity].totalDeposited -= lockPosition.ybShares;

        emit Burn(_to, lockPosition.sglAssetID, _tokenId);
    }

If a user attempts to call unlock after singularity has been removed activeSingularities[_singularity] will return a blank struct, thus making the function later revert.

The user will never be able to reclaim their funds. Even if the singularity is later re-addded totalDeposited will be wiped out upon removal, thus the function will revert due to underflow.

Tools Used

Manual review

Recommended Mitigation Steps

add a function to allow unlocks of locks even after Singularity has been removed.

Assessed type

Error

c4-sponsor commented 3 months ago

0xRektora (sponsor) acknowledged

c4-sponsor commented 3 months ago

0xRektora marked the issue as disagree with severity

0xRektora commented 3 months ago

Probability of this happening is very low. I'd put it as informational.

c4-judge commented 3 months ago

dmvt marked the issue as duplicate of #130

c4-judge commented 3 months ago

dmvt marked the issue as not a duplicate

c4-judge commented 3 months ago

dmvt changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

dmvt marked the issue as grade-b

cryptotechmaker commented 2 months ago

Not a duplicate of #130

c4-judge commented 2 months ago

This previously downgraded issue has been upgraded by dmvt

c4-judge commented 2 months ago

dmvt marked the issue as duplicate of #110

c4-judge commented 2 months ago

dmvt marked the issue as satisfactory