JuggerNaut63 - Excessive Token Withdrawal in ERC20 Bridging Finalization

[sherlock-admin2]

JuggerNaut63

High

Excessive Token Withdrawal in ERC20 Bridging Finalization

Summary

The finalizeBridgeERC20 function in the StandardBridge abstract contract lacks sufficient checks on the deposit balance before decrementing it. This could allow malicious actors to withdraw more tokens than they have deposited, leading to significant financial losses.

Vulnerability Detail

The vulnerability arises from the lack of a proper balance check before decrementing the deposits[_localToken][_remoteToken] mapping.

278:     function finalizeBridgeERC20(
279:         address _localToken,
280:         address _remoteToken,
281:         address _from,
282:         address _to,
283:         uint256 _amount,
284:         bytes calldata _extraData
285:     )
286:         public
287:         onlyOtherBridge
288:     {
289:         require(paused() == false, "StandardBridge: paused");
290:         if (_isOptimismMintableERC20(_localToken)) {
291:             require(
292:                 _isCorrectTokenPair(_localToken, _remoteToken),
293:                 "StandardBridge: wrong remote token for Optimism Mintable ERC20 local token"
294:             );
295: 
296:             OptimismMintableERC20(_localToken).mint(_to, _amount);
297:         } else {
298:@=>          deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] - _amount;
299:             IERC20(_localToken).safeTransfer(_to, _amount);
300:         }
---
304:         _emitERC20BridgeFinalized(_localToken, _remoteToken, _from, _to, _amount, _extraData);
305:     }

The line deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] - _amount; does not verify if the current deposit balance is sufficient to cover _amount. If _amount exceeds the available balance, a malicious actor could manipulate this to withdraw more tokens than they have actually deposited.

A malicious actor could exploit this vulnerability by triggering a call to finalizeBridgeERC20 with an _amount greater than the available deposit balance. Without proper checks, the attacker could withdraw more tokens than they have deposited, leading to unauthorized token transfers.

Impact

Unauthorized withdrawal of tokens could lead to significant financial losses for the contract's stakeholders.

Code Snippet

https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/contracts-bedrock/src/universal/StandardBridge.sol#L278-L305

Tool used

Manual Review

Recommendation

Implement Balance Check: Ensure that the deposit balance is sufficient before performing the subtraction. Add a check to prevent excessive withdrawals.

function finalizeBridgeERC20(
    address _localToken,
    address _remoteToken,
    address _from,
    address _to,
    uint256 _amount,
    bytes calldata _extraData
)
    public
    onlyOtherBridge
{
    require(paused() == false, "StandardBridge: paused");
    if (_isOptimismMintableERC20(_localToken)) {
        require(
            _isCorrectTokenPair(_localToken, _remoteToken),
            "StandardBridge: wrong remote token for Optimism Mintable ERC20 local token"
        );

        OptimismMintableERC20(_localToken).mint(_to, _amount);
    } else {
        uint256 currentDeposit = deposits[_localToken][_remoteToken];
+       require(currentDeposit >= _amount, "StandardBridge: insufficient deposit");

        // Safe subtraction to prevent excessive withdrawal
+       deposits[_localToken][_remoteToken] = currentDeposit - _amount;
        IERC20(_localToken).safeTransfer(_to, _amount);
    }

    _emitERC20BridgeFinalized(_localToken, _remoteToken, _from, _to, _amount, _extraData);
}