hats-finance / Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad

4 stars 4 forks source link

Anyone can call exitPosition for oTAPTokenID they don't own, which would burn that oTAP token. #16

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

Github username: @Emedudu Twitter username: -- Submission hash (on-chain): 0x0694da8c0a552279f970b7690748db75652dd901e77563645df088a5d604a39a Severity: high

Description: Description\ An attacker can call exitPosition on any oTAPTokenID, which would burn the token against the will of the real owner.

Attack Scenario\ TapiocaOptionBroker#exitPosition does not check if the caller is the owner or is approved by the owner of the oTAPTokenID.

After lockDuration of that oTAPPosition has passed, anyone can call exitPosition against intention of the oTAPToken owner. This would burn the oTAP token against the owner's will.

Note that TapiocaOptionBroker is permitted to burn any oTAP token(https://github.com/hats-finance/Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad/blob/dev/contracts/options/oTAP.sol#L118)

As a result of that, the owner of the oTAPTokenID will not be able to call TapiocaOptionBroker#exerciseOption cos the check for ownership will cause a revert

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) TapiocaOptionBroker#exitPosition should have this check:

    // Check allowance.
    {
        address owner = oTAP.ownerOf(_oTAPTokenID);
        // oTAP.isApprovedOrOwner(msg.sender, _oTAPTokenID)
        if (
            owner != msg.sender &&
            !isERC721Approved(
                owner,
                msg.sender,
                address(oTAP),
                _oTAPTokenID
            )
        ) {
            revert NotAuthorized();
        }
    }
0xRektora commented 5 months ago

This is an MEV feature, we want to close other's positions as soon as they are expired.