M-02 Unmitigated

[c4-bot-1]

Lines of code

https://github.com/karak-network/karak-restaking/tree/v2/src/entities/NativeVault.sol#L509-L517

Vulnerability details

Impact

The exchange rate is adjusted unfairly in the _decreaseBalance function when a node owner lacks sufficient shares to burn.

Proof of Concept

When decreasing balance of a node owner, he may have fewer shares than what should be burned. In this case, all of his shares are burned. However, totalAssets is still reduced by the original amount of assets, so the subtracted amount from totalAssets is larger than the amount corresponding to the actually burning shares. As a result, the exchange rate is changed unfairly, resulting in the loss of fund to remaining users.

    function _decreaseBalance(address _of, uint256 assets) internal {
        NativeVaultLib.Storage storage self = _state();
511:    uint256 shares = Math.min(convertToShares(assets), balanceOf(_of));
512:    _beforeWithdraw(assets, shares);
513:    _burn(_of, shares);
        self.totalAssets -= assets;
515:    self.ownerToNode[_of].totalRestakedETH -= assets;
        emit DecreasedBalance(self.ownerToNode[_of].totalRestakedETH);
    }

Consider the following scenario:

1. Alice add a validator with 32 ETH and successfully complete a snapshot, and Bob adds two validators with 32 ETH and successfully complete a snapshot.

   totalAssets : 96

2. Slashing of 6 ETHER occurs.

   totalAssets : 96 - 6 = 90 ETHER convertToAssets(balanceOf(Alice)) : 30 ETHER convertToAssets(balanceOf(Bob)) : 60 ETHER

3. Alice's validator loses all its funds, so totalDeltaWei is -32 ETHER.

4. A snapshot is done for Alice's node.

   totalDeltaWei : -32 ETHER totalAssets : 90 - 32 = 58 ETHER convertToAssets(balanceOf(Bob)) : 58 ETHER (Bob has all remaining shares.)

In the described scenario, Bob incurs an additional loss of 60 - 58 = 2 ETHER (plus 2 ETHER from slashing).

Tools Used

Manual review

Recommended Mitigation Steps

Only the amount corresponding to burning shares should be subtracted from totalAssets.

    function _decreaseBalance(address _of, uint256 assets) internal {
        NativeVaultLib.Storage storage self = _state();
-       uint256 shares = Math.min(convertToShares(assets), balanceOf(_of));
+       uint256 shares = convertToShares(assets);
+       if (shares > balanceOf(_of)) {
+           shares = balanceOf(_of);
+           self.totalAssets -= convertToAssets(shares);
+       }
+       else {
+           self.totalAssets -= assets;
+       }
        _beforeWithdraw(assets, shares);
        _burn(_of, shares);
-       self.totalAssets -= assets;
        self.ownerToNode[_of].totalRestakedETH -= assets;
        emit DecreasedBalance(self.ownerToNode[_of].totalRestakedETH);
    }

Assessed type

Other

[c4-judge]

MiloTruck marked the issue as new finding

[c4-judge]

MiloTruck marked the issue as primary issue

[c4-judge]

MiloTruck marked the issue as duplicate of #16

[c4-judge]

MiloTruck marked the issue as satisfactory