Open howlbot-integration[bot] opened 1 month ago
MiloTruck marked the issue as satisfactory
MiloTruck marked the issue as selected for report
Great find!
The crux of this issue is that nodeBalanceWei
is calculated after _transferToSlashStore()
is called, so node.nodeAddress.balance
will have already decreased before the unattributed balance can be added to totalRestakedETH
.
This causes a loss of funds for the node owner as he does not receive shares for the unattributed balance that was lost, as such, I believe high severity is appropriate.
Lines of code
https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L238 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L277 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L348 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L512
Vulnerability details
Impact
The Karak protocol includes a slashing mechanism that allows the contract owner to penalize stakers by reducing their staked assets in the event of malicious activity by the operator. If a user with a staked balance of 32 ETH is subject to a 3 ETH slashing, they should ideally be able to withdraw the remaining 29 ETH. However, due to a flaw in the implementation, when the user attempts to fully withdraw their ETH, they are only able to withdraw less than the actual remaining amount, with some ETH becoming permanently locked in the protocol.
Specifically, if all share tokens are burned during the withdrawal process, the user cannot access the remaining locked ETH. This results in users receiving fewer ETH than they are entitled to, with the excess ETH becoming inaccessible and permanently locked within the protocol.
Proof of Concept
Consider the following simplified scenario, where Alice want to restake her 32 ETH into Karak protocol, using 1 validator and no rewards acumulated for simplicity. In this example, although Alice initially had 32 ETH staked and should be able to withdraw 29 ETH (after a 3 ETH slashing), she ends up receiving only 26 ETH. The remaining 3 ETH becomes permanently locked in the contract. The steps she need to perform are the following:
Staking and Initial Setup:
createNode
to point her withdrawal credentials to itvalidateWithdrawalCredentials
followed bystartSnapshot
andvalidateSnapshotProofs
.After these actions, the state will look like this:
totalAssets = 32e18
totalSupply = 32e18
totalRestakedETH = 32e18
(in her Node struct)Slashing:
totalAssets
to 29e18.Withdraw from Beacon Chain:
startSnapshot
,validateSnapshotProofs
,startWithdraw
, andfinishWithdraw
.Starting Snapshot
startSnapshot
is executed,_transferToSlashStore
is called, slashing 3 ETH from her node and transferring them toSlashStore
. This reducestotalRestakedETH
to29e18
in her Node struct and saves the new snapshot withnodeBalanceWei = 29e18
.Validate snapshot proofs
validateSnapshotProofs
to validate her balance proofs:balanceDeltaWei = -32e18
(the difference betweennewBalanceWei
of 0 andprevBalanceWei
of 32e18)snapshot.remainigProofs = 0
since Alice has only one validator_updateSnapshot
finalize snapshot due to no remaining active validators, where the balance of the user will be decreased due to the following calculation:29e18 - 32e18 = -3e18
, reducing Alice's shares by 3e18. The state after execution will be:node.totalRestakedETH: 26e18
node.withdrawableCreditedNodeETH: 29e18
balanceOf(alice) = 28689655172413793104
totalAssets = 26e18
totalSupply = 28689655172413793104
Starting Withdrawal
startWithdraw
. There is a check to prevent a user from withdrawing more than they actually can, but in Alice's situation, it will be less than what she should withdraw:withdrawableWei(msg.sender)
will return the minimum between:convertToAssets
is calculated as:(shares * (totalAssets + 1)) / (totalSupply + 1)
which is:(28689655172413793104 * (26e18 + 1)) / (28689655172413793104 + 1) = 26e18
while_state().ownerToNode[nodeOwner].withdrawableCreditedNodeETH
is 29e18 (the actual withdrawable wei after slashing)Finishing Withdrawal:
finishWithdrawal
sends the maximum withdrawableWei (26e18) to Alice, burning all her tokens and causing 3 ETH to remain stuck in the node.Recommended Mitigation Steps
Due to the complexity and the current limitations in testing the implementation of the slashing mechanism, a precise fix is difficult to pinpoint. In the example provided above, the problem occurs at step 5 where
_decreaseBalance
is called again reducing thetotalAssets
by another3e18
(the slashed amount). IftotalAssets
are not decreased again and stays29e18
in the next step ,withdrawableWei(msg.sender)
would correctly return the minimum between28999999999999999999 and 29000000000000000000
. However, to address the root cause of the issue, a better mechanism for handling slashing should be implemented and tested.Assessed type
Other