burst() not return eth when action fails

mediumCode4rena

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/Magnetar.sol#L72

Vulnerability details

Vulnerability details

when execute Magnetar.burst() The user can specify allowFailure = true, and if it fails, ignore the current _action and execute another _action.

burst() -> _executeCall(_allowFailure)

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

        if (_actionValue > 0) {
            (success, returnData) = _target.call{value: _actionValue}(_actionCalldata);
        } else {
            (success, returnData) = _target.call(_actionCalldata);
        }

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

But with the current implementation, if the user also passes _action.value> 0 && _action.allowFailure = true

If the action fails, the _action.value is not returned to the user, it stays in the contract.

Although the owner can retrieve the eth left in the contract via rescueEth(), the eth is not returned to the user until the owner executes rescueEth().

However, before the owner executes rescueEth(), a malicious user can use the contract's eth directly to execute an action .

For example, by using MagnetarAssetModule.depositRepayAndRemoveCollateralFromMarket() -> _withdrawToChain() -> _lzWithdraw() -> sendPacket{value:??} () to use up eth.

Impact

If the action fails, the eth left in the contract can be stolen by malicious people.

Recommended Mitigation

Recommends the final return of all eth

diff📋
    function burst(MagnetarCall[] calldata calls) external payable {
        uint256 valAccumulator;

        uint256 length = calls.length;

        for (uint256 i; i < length; i++) {
.....

        if (msg.value != valAccumulator) revert Magnetar_ValueMismatch(msg.value, valAccumulator);
+       if(address(this).balance>0){
+         msg.sender.call{value:address(this).balance}(""); // return all eth
+       }
      }

Assessed type

Context