[H9] Missing check on helper contract allows arbitrary actions and theft of assets

criticalCode4rena

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/modules/MagnetarOptionModule.sol#L173-L199

Vulnerability details

Impact

The MagnetarOptionModule contract implements the exitPositionAndRemoveCollateral function which allows users to do a series of operations which is irrelevant to the issue. The user passes in the variable data, and later, data.externalData is used to extract out relevant contract addresses. These are then checked against a whitelist.

solidity📋
if (data.externalData.bigBang != address(0)) {
    if (!cluster.isWhitelisted(0, data.externalData.bigBang)) {
        revert Magnetar_TargetNotWhitelisted(data.externalData.bigBang);
    }
}
if (data.externalData.singularity != address(0)) {
    if (!cluster.isWhitelisted(0, data.externalData.singularity)) {
        revert Magnetar_TargetNotWhitelisted(data.externalData.singularity);
    }
}

The main issue is that the data.externalData also has a marketHelper field which is not checked against a whitelist and ends up being used.

solidity📋
(Module[] memory modules, bytes[] memory calls) = IMarketHelper(data.externalData.marketHelper).repay(
    address(this), data.user, false, data.removeAndRepayData.repayAmount
);
(bool[] memory successes, bytes[] memory results) = bigBang_.execute(modules, calls, true);

The helper contracts are used to construct the calldata for market operations. In the above snippet, the helper contract is passed in some data, and it is expected to create a calldata out of the passed in data. The expected output is the repay module and a call value which when executed, will repay for the data.user's account.

However, since the marketHelper contract is never checked against a whitelist, malicious user can pass in any address in that place. So the above call can return any data payload, and the bigBang_.execute will execute it without any checks. This means the malicious helper contract can return a borrow payload of some random user, and the contract will end up borrowing USDO against that user's position. The Magnetar contract is assumed to have approval for market operations, and thus the Magnetar's approval is essentially exploited by the attacker to perform arbitrary actions on any user's account.

This can be used by any user to steal collateral from other user's bigbang position, or borrow out usdo tokens on their position. Since this is direct theft, this is a high severity issue.

Proof of Concept

The absence of checks is evident from the code snippet. Assuming marketHelper contract is malicious, we see that is used in 2 places to create payloads, which must also be deemed malicious.

solidity📋
(Module[] memory modules, bytes[] memory calls) = IMarketHelper(data.externalData.marketHelper).repay(
    address(this), data.user, false, data.removeAndRepayData.repayAmount
);

solidity📋
(Module[] memory modules, bytes[] memory calls) = IMarketHelper(data.externalData.marketHelper)
    .removeCollateral(data.user, removeCollateralTo, collateralShare);

These are then executed, and the Magnetar is assumed to have approvals from users, so these are obviously malicious interactions.

In the other module contracts, the marketHelper is checked against a whitelist, but not in this module. This is a clear oversight. Below is the example from the MagnetarMintCommonModule.

solidity📋
if (!cluster.isWhitelisted(0, marketHelper)) {
    revert Magnetar_TargetNotWhitelisted(marketHelper);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Check the helper contract against a whitelist

Assessed type

Invalid Validation