Submitted by immeas, also found by immeas
https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/Magnetar.sol#L199-L212
https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/Magnetar.sol#L304-L305
This issue is a combination of three other issues:
In short, the first issue describes that, due to how MagnetarAction.TapToken is setup, it will leave the tOLP position a user creates through TapiocaOptionBroker::participate stuck in the Magnetar contract. As it mints the position to msg.sender which will be the Magnetar contract:
The two following ones:
Firstly, describes how anyone can take any tokens that are in the Magnetar contract by granting themselves permissions to transfer any whitelisted tokens in using MagnetarAction.Permit. Because of how MagnetarStorage::_checkSender validates that the first argument in the calldata to MagnetarAction.Permit is the same as msg.sender. This together with that a lot of the calls allowed through MagnetarAction.Permit (IYieldBox::setApprovalForAll, IYieldBox::setApprovalForAsset, IERC20::approve, and IERC721::approve) have address to; i.e. the operator/approvee as the first argument. Hence, any user can approve themselves to transfer tokens out of the contract.
Secondly, by using MagnetarAction.OFT; which allows anyone to transfer tokens out of Magnetar (and steal any approved tokens to Magnetar) since there is an unvalidated call done to any whitelisted contract in MagnetarAction.OFT.
The contract is not in itself supposed to hold any tokens; in itself these issues are not that severe by themselves. However, these issues combined allows an attacker to steal the position completely. Since the first one makes the position be stuck in Magnetar and the two last ones makes it not actually stuck, but retrievable by anyone.
If a user uses MagnetarAction.TapToken to create their position they can have their position and/or rewards stolen.
Test in tap-token/test/Magnetar.t.sol, builds on the test described in MagnetarAction.TapToken integration will leave tokens stuck in Magnetar contract:
Full test with setup can be found here.
Consider implementing the mitigations described in the three mentioned referenced issues:
Invalid Validation
cryptotechmaker (Tapioca) disagreed with severity and commented:
0xRektora (Tapioca) confirmed and commented:
LSDan (judge) decreased severity to Medium
