Insecure Withdrawal Function Allows Unauthorized Access to Funds

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-03-coinbase/blob/e0573369b865d47fed778de00a7b6df65ab1744e/src/MagicSpend/MagicSpend.sol#L169-L176

Vulnerability details

Impact

The lack of validation in the withdrawGasExcess function exposes the contract to the risk of unauthorized fund withdrawals. Malicious actors could exploit this vulnerability to drain the contract of its funds, leading to financial loss or disruption of intended contract functionalities.

Proof of Concept

The withdrawGasExcess function in the MagicSpend contract facilitates the withdrawal of excess ETH associated with a user's account. However, the function lacks proper validation before executing the withdrawal. The function directly calls the internal _withdraw function without performing any validation checks. This internal function is documented with a comment stating that callers must validate the withdrawal legitimacy before calling it, implying that the responsibility for validation lies outside of the function.

function withdrawGasExcess() external {
    uint256 amount = _withdrawableETH[msg.sender];
    // we could allow 0 value transfers, but prefer to be explicit
    if (amount == 0) revert NoExcess();

    delete _withdrawableETH[msg.sender];
    _withdraw(address(0), msg.sender, amount);
}

The absence of validation in the withdrawGasExcess function means that any user can invoke it to withdraw funds without proper authorization or validation. This could potentially lead to unauthorized fund withdrawals, bypassing any intended restrictions or security measures.

Tools Used

Manual

Recommended Mitigation Steps

Proper validation should be added to the withdrawGasExcess function to ensure that only legitimate withdrawals are allowed. This can be achieved by performing checks to verify the legitimacy of the withdrawal request, such as ensuring that the withdrawal amount is valid and that the caller is authorized to withdraw funds.

Assessed type

Access Control

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[raymondfam]

Please visit the readme link on Pay gas and transfer funds during execution. It's supposed to be called by SCW on step 7.

[c4-pre-sort]

raymondfam marked the issue as primary issue

[3docSec]

The function works only if _withdrawableETH[msg.sender]; is non-zero; this means that the caller must have previously been awarded these funds, and this contradicts the any user can invoke it to withdraw funds without proper authorization or validation. statement.

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid