code-423n4 / 2024-03-coinbase-findings

1 stars 0 forks source link

no check for input in 'postOp' #174

Closed c4-bot-5 closed 8 months ago

c4-bot-5 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-coinbase/blob/main/src/MagicSpend/MagicSpend.sol#L143

Vulnerability details

Impact

Detailed description of the impact of this finding. There is no check on actualGasCost.It can be less than maxGasCost. it causes withdrawable to be reduced.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. function postOp(IPaymaster.PostOpMode mode, bytes calldata context, uint256 actualGasCost) external onlyEntryPoint { // PostOpMode.postOpReverted should be impossible. // Only possible cause would be if this contract does not own enough ETH to transfer // but this is checked at the validation step. assert(mode != PostOpMode.postOpReverted);

    (uint256 maxGasCost, address account) = abi.decode(context, (uint256, address));

    // Compute the total remaining funds available for the user accout.
    // NOTE: Take into account the user operation gas that was not consummed.

@> uint256 withdrawable = _withdrawableETH[account] + (maxGasCost - actualGasCost);

    // Send the all remaining funds to the user accout.
    delete _withdrawableETH[account];
    if (withdrawable > 0) {
        SafeTransferLib.forceSafeTransferETH(account, withdrawable, SafeTransferLib.GAS_STIPEND_NO_STORAGE_WRITES);
    }
}

Tools Used

Recommended Mitigation Steps

if(maxGasCost - actualGasCost>0) { uint256 withdrawable = _withdrawableETH[account] + (maxGasCost - actualGasCost); }

Assessed type

Context

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #99

raymondfam commented 8 months ago

See #99.

c4-judge commented 8 months ago

3docSec marked the issue as unsatisfactory: Invalid