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

0 stars 0 forks source link

Loss of funds of new deposits in case bridges were slashed already #239

Closed c4-bot-7 closed 4 months ago

c4-bot-7 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BridgeRouterFacet.sol#L169

Vulnerability details

Impact

 Loss of funds for new depositors in case slashing happened already before their deposits which is unfair. 

Proof of Concept

Looking at the code below in function _ethConversion, the protocol is covering the loss proportionally on withdrawing, according to the comment:

// Accounting for situations when the vault (via bridges) experiences loss in value

function _ethConversion(uint256 vault, uint88 amount) private view returns (uint88) {
        uint256 dethTotalNew = vault.getDethTotal();
        uint88 dethTotal = s.vault[vault].dethTotal;

        if (dethTotalNew >= dethTotal) {
            // when yield is positive 1 deth = 1 eth
            return amount;
        } else {
            // negative yield means 1 deth < 1 eth
            // @dev don't use mulU88 in rare case of overflow
            return amount.mul(dethTotalNew).divU88(dethTotal);
        }

The issue here if bridges balances get slashed (a penalty is done on bridges Steth,Reth in some situations) then dethTotalNew will be less than dethTotal. However, new depositors will incur the loss (proportionally).

In different words, if a new user deposited an amount after a slashed already was done in the bridges, he will lose a partial fund of his deposit when he try to withdraw his deposit. which is unfair to the user since he made the deposit after, not before.

What is pool getting slashed? a reference from rocketpool https://docs.rocketpool.net/guides/node/responsibilities#:~:text=Penalties ​,network%2C it may get slashed.

Validators are penalized for small amounts of ETH if they are offline and fail to perform their assigned duties. This is called leaking. If a validator violates one of the core rules of the Beacon chain and appears to be attacking the network, it may get slashed.

Tools Used

Manual Review

Recommended Mitigation Steps

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

raymondfam commented 5 months ago

Could have been more elaborate and informational.

c4-sponsor commented 5 months ago

ditto-eth (sponsor) disputed

ditto-eth commented 5 months ago

https://dittoeth.com/technical/blackswan#global-black-swan

This is mentioned in the docs but this is just how the system is designed. It's true that someone coming in after an rETH/stETH slashing event would be affected upon withdrawal, but presumably the user would have full knowledge of any slashing event that affects a substantial portion of all staked ETH. Another case of overly conservative behavior to protect the peg

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Out of scope