`MagnetarAction.TapToken` integration will leave tokens stuck in `Magnetar` contract

[c4-bot-4]

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/Magnetar.sol#L304-L305

Vulnerability details

Description

In Magnetar there is the functionality to interact with the TapoiocaOptionBroker and TapiocaOptionLiquidityProvision to manage your options using MagnetarAction.TapToken.

The issue is that if you do this, for the calls ITapiocaOptionBroker.exerciseOption and ITapiocaOptionBroker.participate the resulting tokens that are rewarded/minted will be stuck in the Magnetar contract.

This is because TapiocaOptionBroker.exerciseOption:

File: tap-token/contracts/options/TapiocaOptionBroker.sol

570:        tapOFT.extractTAP(msg.sender, tapAmount);

and TapiocaOptionBroker::participate:

File: tap-token/contracts/options/TapiocaOptionBroker.sol

301:        oTAPTokenID = oTAP.mint(msg.sender, lockExpiry, uint128(target), _tOLPTokenID);

mints to msg.sender which when called from Magnetar will be Magnetar.

Impact

A user using Magnetar for participating or claiming rewards will have their tokens stuck in Magnetar.

Proof of Concept

Test in tap-token/test/Magnetar.t.sol:

    function testMagnetarParticipateOTAPStuckInMagnetar() public {
        MagnetarCall memory permitYieldBox = MagnetarCall({
            id: MagnetarAction.Permit,
            target: address(pearlmit),
            value: 0,
            allowFailure: false,
            call: abi.encodeWithSelector(
                IPearlmit.approve.selector,
                address(yieldbox), 1, address(tOLP), type(uint200).max, type(uint48).max
            )
        });

        MagnetarCall memory lock = MagnetarCall({
            id: MagnetarAction.TapToken,
            target: address(tOLP),
            value: 0,
            allowFailure: false,
            call: abi.encodeWithSelector(
                TapiocaOptionLiquidityProvision.lock.selector,
                address(magnetar), singularity, 1 weeks, 1e18
            )
        });

        MagnetarCall memory permitTOLPToPearlmit = MagnetarCall({
            id: MagnetarAction.Permit,
            target: address(tOLP),
            value: 0,
            allowFailure: false,
            call: abi.encodeWithSelector(IERC721.setApprovalForAll.selector, address(pearlmit), true)
        });

        MagnetarCall memory permitPearlmitToTOB = MagnetarCall({
            id: MagnetarAction.Permit,
            target: address(pearlmit),
            value: 0,
            allowFailure: false,
            call: abi.encodeWithSelector(
                IPearlmit.approve.selector,
                address(tOLP), 1, address(tOB), 1, type(uint48).max
            )
        });

        MagnetarCall memory participate = MagnetarCall({
            id: MagnetarAction.TapToken,
            target: address(tOB),
            value: 0,
            allowFailure: false,
            call: abi.encodeWithSelector(TapiocaOptionBroker.participate.selector, 1)
        });

        MagnetarCall[] memory calls = new MagnetarCall[](5);
        calls[0] = permitYieldBox;
        calls[1] = lock;
        calls[2] = permitTOLPToPearlmit;
        calls[3] = permitPearlmitToTOB;
        calls[4] = participate;

        magnetar.burst(calls);

        assertEq(oTAP.ownerOf(1),address(magnetar));
    }

Full test setup can be found here.

Tools Used

Manual audit

Recommended Mitigation Steps

Consider adding the ability for the caller to declare a receiver in TapiocaOptionBroker::participate and exerciseOption. Similar to how it's done in TapiocaOptionLiquidityProvision::lock.

Assessed type

Token-Transfer

[cryptotechmaker]

Somehow duplicate of https://github.com/code-423n4/2024-02-tapioca-findings/issues/176

[c4-judge]

dmvt marked the issue as duplicate of #176

[c4-judge]

dmvt changed the severity to 2 (Med Risk)

[c4-judge]

dmvt marked the issue as satisfactory