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

0 stars 0 forks source link

User cant withdraw ETH if user enter more than ethEscrowed in withdraw function #268

Closed c4-bot-9 closed 3 months ago

c4-bot-9 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L63-L75

Vulnerability details

Impact

when user try to withdraw eth from the bridge and if user enters dethamount as more than eth Escrowed and user has also credit balance in other bridge then it reverts. There is a logical error in the line instance-1: https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L63 instance-2: https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L93

Every time user enter deth amount more than then the transaction is going to revert.

Proof of Concept

In withdraw function in BridgeRouter.sol , if Vault == Vault.One then it calls for the function assessDeth function (which is gives information about eth which is not covered by the bridge credit). In assessDeth function, when deth is more then ethEscrowed in one bridge then it checks for the ethEscrowed in other bridge code : if (creditReth >= amount) { VaultUser.bridgeCreditReth -= amount; return 0; }

        VaultUser.bridgeCreditReth = 0;
        amount -= creditReth;
        creditSteth = VaultUser.bridgeCreditSteth;
        if (creditSteth < C.ROUNDING_ZERO) {
            // Valid withdraw when no STETH credits
            return amount;
        } else {
            if (IBridge(stethBridge).getDethValue() < C.ROUNDING_ZERO) // Validation ERROR
                 if (creditSteth >= amount) {
                    VaultUser.bridgeCreditSteth -= amount;
                    return 0;
                } else {
                    VaultUser.bridgeCreditSteth = 0;
                    return amount - creditSteth;
                }
            } else {
                // Must use available bridge credits on withdrawal
                // @dev Prevents abusing bridge for arbitrage
                revert Errors.MustUseExistingBridgeCredit();
            }

In the line 11 there is a logical error. there always exist eth on the stethBridge which is more than the C.ROUNDING_ZERO so , else block is executed every time user enters deth amount greater than the bridge credits in one bridge and also , if user has other bridge credits. if user has enough steth credit and after it checks for the if bridge has low deth locked in it ( which is always false) so it goes to else block which is a revert statement. so the transaction reverts.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the logic of assessdeth function to check for minimum deth locked in eth by changing to if (IBridge(stethBridge).getDethValue() > C.ROUNDING_ZERO)

Assessed type

Invalid Validation

c4-pre-sort commented 3 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 3 months ago

raymondfam marked the issue as primary issue

raymondfam commented 3 months ago

Insufficient proof to support your claim. Additionally, the reverting logic is associated with a nested else clause.

c4-judge commented 3 months ago

hansfriese marked the issue as unsatisfactory: Insufficient proof