c4-bot-5 commented 2 months ago

Lines of code

Vulnerability details

H-01: Slashing NativeVault will lead to locked ETH for the users

Comments

The original implementation calculated slashed amounts and transferred funds before checking the user’s latest balance. This process failed to account for any unattributed balances, potentially resulting in locked or lost funds for users.

Mitigation

Fix link

The mitigation addresses this issue by modifying the _burnSlashed function to slash only the ETH that has already been accounted for (node.withdrawableCreditedNodeETH), thereby preventing scenarios where unattributed balances could be incorrectly included in the slashing calculations.

    function _burnSlashed(address nodeOwner) internal {
        NativeVaultLib.Storage storage self = _state();
        NativeVaultLib.NativeNode storage node = self.ownerToNode[nodeOwner];

        uint256 slashedAssets;
        // Account for rounding errors which might make convertToAssets() > totalRestakedETH
        if (node.totalRestakedETH > convertToAssets(balanceOf(nodeOwner))) {
            // slashed ETH = total restaked ETH (node + beacon) - share price equivalent ETH
            slashedAssets = node.totalRestakedETH - convertToAssets(balanceOf(nodeOwner));
        }

        // sweepable ETH = min(ETH available on node that has been minted shares for, total slashed ETH)
        uint256 slashedWithdrawable = Math.min(node.withdrawableCreditedNodeETH, slashedAssets);

        // withdraw sweepable ETH to slashStore
        INativeNode(node.nodeAddress).withdraw(address(0), slashedWithdrawable);

        // update total restaked ETH available (node + beacon)
        node.totalRestakedETH -= slashedWithdrawable;
        node.withdrawableCreditedNodeETH -= slashedWithdrawable;
    }

Conclusion

The mitigation effectively prevents the issue by ensuring that only credited balances are considered during slashing.

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory