H-01 MitigationConfirmed

[c4-bot-1]

Lines of code

Vulnerability details

The fix applied by the team fully mitigates H-01.

[alcueca]

The mitigation review should include more than just links to the issue and the fix. Not much is needed, but at least a description of both.

[c4-judge]

alcueca marked the issue as unsatisfactory: Insufficient quality

[s1n1st3r01]

Original vulnerability

---

Withdrawing funds in the protocol is a two-step process. First you request a withdrawal by calling the function WithdrawQueue::withdraw(), then you wait a specific amount of time specified by the state coolDownPeriod in WithdrawQueue.sol, before being able to claim your funds by calling the function WithdrawQueue::claim() and supplying it with your withdraw request's index in the calldata.

The issue is that the WithdrawQueue::claim() function sends the native ETH funds to the user using the transfer() function, rather than call(). The issue with transfer() is that it only forwards 2,300 gas which is usually not enough if the recipient is a smart contract executing non-trivial logic in the receive() or fallback() function, with multisig wallets being a great example of that. Gnosis safe for example consumes at least > 6k gas for the call to reach the implementation contract and emit an event.

    function claim(uint256 withdrawRequestIndex) external nonReentrant {
        .....................

        // send selected redeem asset to user
------> if (_withdrawRequest.collateralToken == IS_NATIVE) {
            payable(msg.sender).transfer(_withdrawRequest.amountToRedeem);
        } else {
            ...............
        }
        ..........
    }
}

transfer() not sending enough gas means that the call will keep reverting, and therefore there will be no way for the user to get his funds back. They'll be permanently stuck.

Mitigation Analysis

---

The mitigation successfully addresses this issue by replacing using call() instead of transfer() to send native ETH to the user trying to claim his funds. call() forwards all available gas which allows the execution flow to continue smoothly and not end up reverting due to insufficient gas.

if (_withdrawRequest.collateralToken == IS_NATIVE) {
-             payable(msg.sender).transfer(_withdrawRequest.amountToRedeem);
+             (bool success, ) = payable(msg.sender).call{ value: _withdrawRequest.amountToRedeem }(
+                 ""
+             );
+             if (!success) revert TransferFailed();
} else {
    .......

[c4-judge]

alcueca marked the issue as satisfactory