code-423n4 / 2024-07-karak-findings

0 stars 0 forks source link

MAX_SLASHING_PERCENT_WAD can be exceeded if a NativeVault's balance is unstable #66

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L79-L92

Vulnerability details

Summary

MAX_SLASHING_PERCENT_WAD can be exceeded if a NativeNode balance goes down due to withdrawn rewards from staking on the beacon chain. It is possible that the Veto committee won't be able to avoid it. This would also happen every time a slashing request is done with the maximum allowed percentage and the balance drops.

Vulnerability Details

fetchEarmarkedStakes in SlasherLib.sol is called when requesting a slash, and we have to wait 2 days after that until the finalizeSlashing() in Core.sol could be called.

    function requestSlashing(
        CoreLib.Storage storage self,
        IDSS dss,
        SlashRequest memory slashingMetadata,
        uint256 nonce
    ) external returns (QueuedSlashing memory queuedSlashing) {
        validateRequestSlashingParams(self, slashingMetadata, dss);
@>>     uint256[] memory earmarkedStakes = fetchEarmarkedStakes(slashingMetadata);

        queuedSlashing = QueuedSlashing({
            dss: dss,
            timestamp: uint96(block.timestamp),
            operator: slashingMetadata.operator,
            vaults: slashingMetadata.vaults,
@>>         earmarkedStakes: earmarkedStakes,
            nonce: nonce
        });
       ............. more code
    function fetchEarmarkedStakes(SlashRequest memory slashingMetadata)
        internal
        view
        returns (uint256[] memory earmarkedStakes)
    {
        earmarkedStakes = new uint256[](slashingMetadata.vaults.length);
        for (uint256 i = 0; i < slashingMetadata.vaults.length; ++i) {
            earmarkedStakes[i] = Math.mulDiv(
                slashingMetadata.slashPercentagesWad[i],
@>>             IKarakBaseVault(slashingMetadata.vaults[i]).totalAssets(),
@>>             Constants.MAX_SLASHING_PERCENT_WAD
            );
        }
    }

But if in the next 2 days the total assets of the NativeVault have gone down due to awards withdrawn or validators opting out, the slash request that has been submitted by the DSS could end up overriding the MAX_SLASHING_PERCENT_WAD and slashing a higher percentage of the total assets, as the finalizeSlashing in SlasherLib.sol doesn't currently validate the balance and just slashes the amount from the slashing request:


    function finalizeSlashing(CoreLib.Storage storage self, QueuedSlashing memory queuedSlashing) external {
        bytes32 slashRoot = calculateRoot(queuedSlashing);
        if (!self.slashingRequests[slashRoot]) revert InvalidSlashingParams();
        if (queuedSlashing.timestamp + Constants.SLASHING_VETO_WINDOW > block.timestamp) {
            revert MinSlashingDelayNotPassed();
        }
        delete self.slashingRequests[slashRoot];

        for (uint256 i = 0; i < queuedSlashing.vaults.length; i++) {
            IKarakBaseVault(queuedSlashing.vaults[i]).slashAssets(
  @>>           queuedSlashing.earmarkedStakes[i],
                self.assetSlashingHandlers[IKarakBaseVault(queuedSlashing.vaults[i]).asset()]
            );
        }
        IDSS dss = queuedSlashing.dss;

        HookLib.callHookIfInterfaceImplemented({
            dss: dss,
            data: abi.encodeWithSelector(dss.finishSlashingHook.selector, queuedSlashing.operator),
            interfaceId: dss.finishSlashingHook.selector,
            ignoreFailure: true,
            hookCallGasLimit: self.hookCallGasLimit,
            supportsInterfaceGasLimit: self.supportsInterfaceGasLimit,
            hookGasBuffer: self.hookGasBuffer
        });
    }

Concept Scenario:

If the initial value of a vault is 3200 ETH (i.e., 100 validators) with a slashing limit of 20% (640 ETH), and if the DSS enforces the maximum slashing percentage, several issues could arise if at least one validator withdraws their stake before the slashing is finalized:

This scenario is likely if the withdrawal request precedes the slashing request, given the 9-day withdrawal window. It could be worsened by multiple withdrawals or other reductions in the vault’s balance, such as slashing from the beacon chain, being finalised within the 2-day SLASHING_VETO_WINDOW.

This issue would consistently occur whenever the DSS opts for maximum slashing and the vault’s balance decreases thereafter. Its also possible if the slashing % is close to the maximum, or if the balance drops by a lot.

The Veto Committee might cancel a slashing request, but this only postpones the inevitable if the vault's balance remains unstable. Moreover, if the DSS insists on maximum slashing, the Veto Committee’s influence is limited, as the only solution would be the DSS itself to NOT slash the max %, which will resulting in over-slashing due to subsequent balance reductions.

Tools Used

Manual review

Recommendations

When finalizing the slashing request and performing the slash for the NativeVault, ensure that the total assets to be slashed do not exceed the MAX_SLASHING_PERCENT_WAD. If they do, simply adjust the amount to match the MAX_SLASHING_PERCENT_WAD.

Assessed type

Context

c4-judge commented 2 months ago

MiloTruck marked the issue as not a duplicate

c4-judge commented 2 months ago

MiloTruck marked the issue as duplicate of #17

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory

c4-judge commented 2 months ago

MiloTruck marked the issue as duplicate of #7

c4-judge commented 2 months ago

MiloTruck marked the issue as not a duplicate

c4-judge commented 2 months ago

MiloTruck marked the issue as duplicate of #17

c4-judge commented 2 months ago

MiloTruck removed the grade

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory

c4-judge commented 2 months ago

MiloTruck marked the issue as unsatisfactory: Invalid

c4-judge commented 1 month ago

MiloTruck removed the grade

c4-judge commented 1 month ago

MiloTruck marked the issue as satisfactory