Submitted by dirk_y, also found by Madalad, bin2chen, kutugu, Ack, 0xStalin (1, 2), 0xTheC0der, cergyk (1, 2), rvierdiiev, and erebus
https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2.sol#L622-L635 
https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2Storage.sol#L336-L338 
https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/modules/MagnetarMarketModule.sol#L70 
https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/modules/MagnetarMarketModule.sol#L212-L241 
https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L84-L91
The MagnetarV2.sol contract is a helper contract that allows users to interact with other parts of the Tapioca ecosystem. In order for Magnetar to be able to perform actions on behalf of a user, the user has to approve the contract as an approved spender (or equivalent) of the relevant tokens in the part of the Tapioca ecosystem the user wants to interact with.
In order to avoid abuse, many of the actions that Magnetar can perform are protected by a check that the owner of the position/token needs to be the msg.sender of the user interacting with Magnetar. However, there are some methods that are callable through Magnetar that dont have this check. This allows a malicious user to use approvals other users have made to Magnetar to steal their underlying tokens.
As I mentioned above, many of the Magnetar methods have a check to ensure that the msg.sender is the from address for the subsequent interactions with other parts of the Tapioca ecosystem. This check is performed by the _checkSender method:
This function does what it is designed to do, however there are some methods that dont include this protection when they should.
One example is the MARKET_BUY_COLLATERAL action that allows a user to buy collateral in a market:
In the market contract there is an underlying call to check whether the sender has the allowance to buy collateral:
Since the msg.sender from the perspective of the market is Magnetar, the user would need to provide a borrow allowance to Magnetar to perform this action through Magnetar.
However, you can see above in the MARKET_BUY_COLLATERAL code snippet that there is no call to _checkSender. As a result, a malicious user can now pass in an arbitrary data.from address to use the allowance provided by another user to perform an unauthorised action. In this case, the malicious user could lever up the users position to increase the users LTV and therefore push the user closer to insolvency; at which point the user can be liquidated for a profit.
Another example of this issue is with the depositRepayAndRemoveCollateralFromMarket method in MagnetarMarketModule.sol. In this instance a malicious user can drain approved tokens from any other user by depositing into the Magnetar yield box:
This is a small snippet from the underlying _depositRepayAndRemoveCollateralFromMarket method that doesnt include a call to _checkSender and therefore the malicious user can simply set extractFromSender to false and specify an arbitrary user address.
The _checkSender method should be used in every method in MagnetarV2.sol and MagnetarMarketModule.sol if it isnt already.
0xRektora (Tapioca) confirmed via duplicate issue 106
