search

sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

cergyk - UsdoMarketReceiverModule::removeAssetReceiver msg_.externalData.marketHelper is unchecked enabling arbitrary market actions from magnetar #48

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 6 months ago

cergyk

high

UsdoMarketReceiverModule::removeAssetReceiver msg_.externalData.marketHelper is unchecked enabling arbitrary market actions from magnetar

Summary

Magnetar is an approved operator/periphery to simplify actions on user accounts. As such it has privileged access to accounts, and ensures that an unauthorized user can not access another account by using _checkSender checks.

However in this check any address in the cluster, is also authorized on any users account.

Unfortunately UsdoMarketReceiverModule::removeAssetReceiver does not check the source chain sender, and does not check a target address (msg_.externalData.marketHelper), which ultimately allows any user to make an arbitrary action on the behalf of any other on any market.

Vulnerability Detail

See that msg_.externalData.marketHelper is unchecked here: https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/usdo/modules/UsdoMarketReceiverModule.sol#L214-L216

UsdoMarketReceiverModule::removeAssetReceiver does not check that msg.user == _srcChainSender, since it ignores the _srcChainSender parameter: https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/usdo/modules/UsdoReceiver.sol#L79

As a result inside Magnetar, an arbitrary call can be made on any market, on behalf of any user, by any other: https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/modules/MagnetarOptionModule.sol#L197-L199

In particular, a user can make the call to removeCollateral on behalf of another, and steal his funds

Impact

Any user can drain any other user's account

Code Snippet

Tool used

Manual Review

Recommendation

Check msg.user == _srcChainSender and that msg_.externalData.marketHelper is whitelisted

Duplicate of #90

cryptotechmaker commented 6 months ago

Low; Module is checked on market at execute. Allowance is also checked on each individual market operation. However we'll add the msg_.externalData.marketHelper whitelist check.

Duplicate of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/75

sherlock-admin2 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

POC would have helped