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

0 stars 0 forks source link

Holding both rEth and stEth can lead to funds getting locked in the protocol #258

Closed c4-bot-4 closed 6 months ago

c4-bot-4 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L39-L109 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BridgeRouterFacet.sol#L101-L123

Vulnerability details

Summary

The ability to withdraw deposited funds and accrued yield is core functionality of the protocol; depositors will be unable to do so under certain conditions.

Vulnerability Details

assessDeth() is called by withdraw() for VAULT.ONE withdrawals to calculate how much of the specified withdrawal, dethAmount, may be subject to fees. This function also prevents a requester, who holds both creditReth & creditSteth, from withdrawing anything above their credits, i.e. yield, until they have first withdrawn those credits. So a user's yield is essentially stuck in the protocol until all credits have been withdrawn.

If user is unable to withdraw one of those credits, because a bridge has less dethValue available than the user has credit, their yield is stuck in the protocol.

>>> if (IBridge(stethBridge).getDethValue() < C.ROUNDING_ZERO) {
        // Can withdraw RETH using STETH credit when STETH bridge is empty
        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();
    }

This scenario can arise because, while user's are prevented from playing arbitrage on their credits, the protocol still permits arbitrage on yield. Depositors with both tokens can choose which token to withdraw their yield in. If more of one is being withdrawn than the other, some users will be left unable to withdraw.

POC

Bob deposits 10 rETH => 10 creditReth

Bob earns 1e18 yield => 10 creditReth + 1e18 yield

Bob deposits 10 stETH => 10 creditReth + 10 creditstEth + 1e18 yield

Bob withdraws 10 reth => 10 creditstEth + 1e18 yield

Bob tries to withdraw 11 stEth but reverts because the bridge does only holds 5 stEth

Bob's yield is stuck in the protocol; he can withdraw maximum 5 stEth until more funds are deposited into the bridge.

Impact

User's with both credits can have their funds locked in the protocol indefinitely if one bridge is low.

Tools Used

Manual Review Foundry Testing

Recommendations

Similarly to how credits work, track yieldSteth and yieldReth allowing user's only withdraw yield in the token in which it was earned unless the vault in which it was earned is empty.

Assessed type

Other

c4-pre-sort commented 6 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 6 months ago

raymondfam marked the issue as duplicate of #40

c4-pre-sort commented 6 months ago

raymondfam marked the issue as duplicate of #119

c4-judge commented 5 months ago

hansfriese marked the issue as unsatisfactory: Invalid