code-423n4 / 2024-02-tapioca-findings

1 stars 1 forks source link

Magnetar Calling arbitrary approved target means calling self - Calling self allows impersonation #79

Closed c4-bot-5 closed 7 months ago

c4-bot-5 commented 7 months ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/Magnetar.sol#L325-L333

Vulnerability details

Impact

Magnetar has added the ability of performing arbitrary calls

The only protection is that the contract must be whitelisted by Cluster

It stands to reason that Magnetar itself will be whitelisted since Cluster is a singleton

We can see that the only validation is for cluster.isWhitelisted

https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/Magnetar.sol#L325-L333

    function _processOFTOperation(
        address _target,
        bytes calldata _actionCalldata,
        uint256 _actionValue,
        bool _allowFailure
    ) private {
        if (!cluster.isWhitelisted(0, _target)) revert Magnetar_NotAuthorized(_target, _target);
        _executeCall(_target, _actionCalldata, _actionValue, _allowFailure);
    }

meaning we can call Magnetar since it will be whitelisted, in _executeCall

https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/Magnetar.sol#L338-L353

function _executeCall(address _target, bytes calldata _actionCalldata, uint256 _actionValue, bool _allowFailure)
        private
    {
        bool success;
        bytes memory returnData;

        if (_actionValue > 0) { /// @audit Arbitrary Execution, we can steal everything here
            (success, returnData) = _target.call{value: _actionValue}(_actionCalldata);
        } else {
            (success, returnData) = _target.call(_actionCalldata);
        }

        if (!success && !_allowFailure) {
            _getRevertMsg(returnData);
        }
    }

Due to the above, We can perform an OFTOperation with Magnetar as the target, and we can simply call burst as Magnetar itself, allowing us to execute any arbitrary operation as authorized.

Effectively sweeping any token, claiming all rewards, etc..

POC

This, in conjunction with the arbitrary external calls, allows sidestepping and impersonating every user in Magnetar

Mitigation

This is one instance of a broader issues that I don't believe we have been given sufficient information to fully investigate

In order for whitehats to fully investigate the possible set of arbitrary combinations, we'd need the full list of all addresses that are whitelisted.

As any single address allowing for ACE will allow stealing all funds from Magnetar, even if the arbitrary call shown in this instance where to be patched.

Immediate Suggested Mitigation

I don't believe we have full information to suggest a safe remediation that will work 100% of the time

The first step towards a safe remediation is to change to be:

mapping(address caller => mapping(address receiver => mapping(bytes4 funSig => bool enabled))

Meaning it will enforce a flow of calls and funsig

In my opinion anything less specific and granular can, over time, open up the system to exploits due to the combinatorial nature of a simple "isAllowed" system

Assessed type

Invalid Validation

c4-judge commented 7 months ago

dmvt marked the issue as primary issue

cryptotechmaker commented 7 months ago

Duplicate of https://github.com/code-423n4/2024-02-tapioca-findings/issues/170

c4-judge commented 7 months ago

dmvt marked the issue as duplicate of #170

c4-judge commented 7 months ago

dmvt marked the issue as duplicate of #100

c4-judge commented 7 months ago

dmvt marked the issue as satisfactory