Error in Slashing calculation causes users to lose double the intended amount

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L425-L446
https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L482-L484
https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L512-L513
https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/NativeVaultLib.sol#L127-L129

Vulnerability details

Impact

A slashing scenario in which users would lose more(double) than the intended slashed amount.

Proof of Concept

Assume the following scenario (for mathematical simplicity assuming that BOB is the first and the only user of the vault):

1. Bob creates a node, and adds 2 validators each with 32ETH as balance
   (Bob.totalRestakedEth = 64ETH, self.totalAssets = 64ETH)

2. The NativeVault is slashed for 30ETH
   (self.totalAssets = 64 - 34 = 34ETH)

3. Bob withdraws from the validators and thus the funds are transferred to the native node
   (bobs_node.balance = 64ETH)

4. Bob calls the startSnapshot() function, here the following things are calculated:
   In the _transferToSlashStore() function:(https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L425-L446)
   (30ETH is transferred from Bob's native node to slashstore,
   Bobs totalRestakedETH = 34ETH)

5. Bob calls the validateSnapshotProofs() function, here the following things are calculated:
   In the _updateSnapshot() function: (https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L482-L484)
   (totalDeltaWei = 34 + (-64) = -30ETH)

In the _decreaseBalance() function called thereafter:(https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L512-L513)
(self.totalAssets = 34 - 30 = 4ETH,
self.ownerToNode[_of].totalRestakedETH = 34 - 30 = 4ETH)

This is wrong since, only 30ETH should have been reduced from the initial 64ETH, but here we can see that 30*2 = 60ETH is being reduced from the user.

Tools Used

Manual

Recommended Mitigation Steps

A different approach may have to be implemented for the fully withdrawal condition.(Many changes would have to be brought here: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/NativeVaultLib.sol#L127-L129)

Assessed type

Math

[10xhash]

The report shows the scenario where an user looses additional assets incase a slashing occurs before the startSnapshot function has been invoked to account the beacon chain updates. Although the report lacks in explanation, the underlying issue is the same as what is reported in code-423n4/2024-07-karak-findings#29

[MiloTruck]

Agree it's a duplicate, but please write a more detailed explanation for future issues.

[10xhash]

Agree it's a duplicate, but please write a more detailed explanation for future issues.

Surely will. Thank you