Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Every operator is unexpectedly DOS'ed from staking to a DSS and possible leveraging for at least 9 days #19

Open
c4-bot-5 opened this issue Jul 30, 2024 · 3 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a insufficient quality report This report is not of sufficient quality QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_20_group AI based duplicate group recommendation

Comments

@c4-bot-5
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/53eb78ebda718d752023db4faff4ab1567327db4/src/Core.sol#L146-L153
https://github.com/code-423n4/2024-07-karak/blob/d19a4de35bcaf31ccec8bccd36e2d26594d05aad/src/entities/Operator.sol#L111-L133
https://github.com/code-423n4/2024-07-karak/blob/d19a4de35bcaf31ccec8bccd36e2d26594d05aad/src/entities/Operator.sol#L89-L101
https://github.com/code-423n4/2024-07-karak/blob/53eb78ebda718d752023db4faff4ab1567327db4/src/Vault.sol#L78-L119
https://github.com/code-423n4/2024-07-karak/blob/d19a4de35bcaf31ccec8bccd36e2d26594d05aad/src/entities/VaultLib.sol#L24-L38

Vulnerability details

Impact

The following Core.finalizeUpdateVaultStakeInDSS function calls the Operator.validateAndUpdateVaultStakeInDSS function. Because the Operator.validateAndUpdateVaultStakeInDSS function calls the Operator.validateQueuedStakeUpdate function, which reverts if queuedStakeUpdate.startTimestamp + Constants.MIN_STAKE_UPDATE_DELAY > block.timestamp is true, the operator is subject to the MIN_STAKE_UPDATE_DELAY for both the DSS allocation and DSS unallocation. Yet, according to https://github.com/code-423n4/2024-07-karak?tab=readme-ov-file#operators, The Operator is subject to a ``MIN_STAKE_UPDATE_DELAY`` of 9 days to prevent front-running a slashing event only for DSS Unallocation; thus, the operator should be allowed to allocate its funds to the DSS without waiting for the MIN_STAKE_UPDATE_DELAY. As a result, every operator is unexpectedly DOS'ed from staking to a DSS and possible leveraging for at least 9 days.

https://github.com/code-423n4/2024-07-karak/blob/53eb78ebda718d752023db4faff4ab1567327db4/src/Core.sol#L146-L153

    function finalizeUpdateVaultStakeInDSS(Operator.QueuedStakeUpdate memory queuedStake)
        external
        nonReentrant
        whenFunctionNotPaused(Constants.PAUSE_CORE_FINALIZE_STAKE_UPDATE)
    {
        _self().validateAndUpdateVaultStakeInDSS(queuedStake);
        emit FinishedStakeUpdate(queuedStake);
    }

https://github.com/code-423n4/2024-07-karak/blob/d19a4de35bcaf31ccec8bccd36e2d26594d05aad/src/entities/Operator.sol#L111-L133

    function validateAndUpdateVaultStakeInDSS(CoreLib.Storage storage self, QueuedStakeUpdate memory queuedStakeUpdate)
        external
    {
        State storage operatorState = self.operatorState[queuedStakeUpdate.operator];
        validateQueuedStakeUpdate(operatorState, queuedStakeUpdate);
        updateVaultStakeInDSS(
            operatorState,
            queuedStakeUpdate.updateRequest.vault,
            queuedStakeUpdate.updateRequest.dss,
            queuedStakeUpdate.updateRequest.toStake
        );
        delete operatorState.pendingStakeUpdates[queuedStakeUpdate.updateRequest.vault];
        IDSS dss = queuedStakeUpdate.updateRequest.dss;
        HookLib.callHookIfInterfaceImplemented({
            dss: dss,
            data: abi.encodeWithSelector(dss.finishUpdateStakeHook.selector, msg.sender),
            interfaceId: dss.finishUpdateStakeHook.selector,
            ignoreFailure: true,
            hookCallGasLimit: self.hookCallGasLimit,
            supportsInterfaceGasLimit: self.supportsInterfaceGasLimit,
            hookGasBuffer: self.hookGasBuffer
        });
    }

https://github.com/code-423n4/2024-07-karak/blob/d19a4de35bcaf31ccec8bccd36e2d26594d05aad/src/entities/Operator.sol#L89-L101

    function validateQueuedStakeUpdate(State storage operatorState, QueuedStakeUpdate memory queuedStakeUpdate)
        internal
        view
    {
        if (queuedStakeUpdate.startTimestamp + Constants.MIN_STAKE_UPDATE_DELAY > block.timestamp) {
            revert OperatorStakeUpdateDelayNotPassed();
        }
        if (
            calculateRoot(queuedStakeUpdate) != operatorState.pendingStakeUpdates[queuedStakeUpdate.updateRequest.vault]
        ) {
            revert InvalidQueuedStakeUpdateInput();
        }
    }

Moreover, as a comparison, the deposits into a vault are not subject to the MIN_WITHDRAW_DELAY of 9 days as shown by the Vault contract's deposit and mint functions, and only the withdrawals from a vault are subject to such MIN_WITHDRAW_DELAY based on the VaultLib.validateQueuedWithdrawal function but each operator must always wait for the MIN_STAKE_UPDATE_DELAY of 9 days no matter it allocates its funds to or unallocates its funds from a DSS.

https://github.com/code-423n4/2024-07-karak/blob/53eb78ebda718d752023db4faff4ab1567327db4/src/Vault.sol#L78-L119

    function deposit(uint256 assets, address to)
        public
        override(ERC4626, IVault)
        whenFunctionNotPaused(Constants.PAUSE_VAULT_DEPOSIT)
        nonReentrant
        returns (uint256 shares)
    {
        if (assets == 0) revert ZeroAmount();
        return super.deposit(assets, to);
    }
    ...
    function deposit(uint256 assets, address to, uint256 minSharesOut)
        external
        nonReentrant
        whenFunctionNotPaused(Constants.PAUSE_VAULT_DEPOSIT_SLIPPAGE)
        returns (uint256 shares)
    {
        if (assets == 0) revert ZeroAmount();
        shares = super.deposit(assets, to);
        if (shares < minSharesOut) revert NotEnoughShares();
    }
    ...
    function mint(uint256 shares, address to)
        public
        override(ERC4626, IVault)
        whenFunctionNotPaused(Constants.PAUSE_VAULT_MINT)
        nonReentrant
        returns (uint256 assets)
    {
        if (shares == 0) revert ZeroShares();
        assets = super.mint(shares, to);
    }

https://github.com/code-423n4/2024-07-karak/blob/d19a4de35bcaf31ccec8bccd36e2d26594d05aad/src/entities/VaultLib.sol#L24-L38

    function validateQueuedWithdrawal(State storage self, bytes32 withdrawalKey)
        internal
        view
        returns (WithdrawLib.QueuedWithdrawal memory qdWithdrawal)
    {
        qdWithdrawal = self.withdrawalMap[withdrawalKey];

        if (qdWithdrawal.start == 0) {
            revert WithdrawalNotFound();
        }

        if (qdWithdrawal.start + Constants.MIN_WITHDRAWAL_DELAY > block.timestamp) {
            revert MinWithdrawDelayNotPassed();
        }
    }

Proof of Concept

The following steps can occur for the described scenario.

  1. An operator calls the Core.requestUpdateVaultStakeInDSS function for multiple times for staking one of its vaults to multiple DSSes.
  2. Just after the operator's Core.requestUpdateVaultStakeInDSS transactions are executed, the operator starts to call the Core.finalizeUpdateVaultStakeInDSS function for finalizing its DSS allocation requests so it can perform leveraging soon.
  3. The operator's Core.requestUpdateVaultStakeInDSS transaction reverts because the MIN_STAKE_UPDATE_DELAY of 9 days is not passed yet even though the operator's DSS allocation should not be subject to such MIN_STAKE_UPDATE_DELAY.
  4. After being unexpectedly DOS'ed for staking to the DSSes and leveraging for 9 days, the operator's DSS allocation requests can finally be finalized by calling the Core.finalizeUpdateVaultStakeInDSS function again.

Tools Used

Manual Review

Recommended Mitigation Steps

The Core.finalizeUpdateVaultStakeInDSS function can be updated to allow the operator's DSS allocation request to be finalized without waiting for the MIN_STAKE_UPDATE_DELAY.

Assessed type

Other

@c4-bot-5 c4-bot-5 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jul 30, 2024
c4-bot-3 added a commit that referenced this issue Jul 30, 2024
@c4-bot-13 c4-bot-13 added the 🤖_20_group AI based duplicate group recommendation label Jul 30, 2024
@howlbot-integration howlbot-integration bot added the insufficient quality report This report is not of sufficient quality label Aug 1, 2024
@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Aug 11, 2024
@c4-judge
Copy link
Contributor

MiloTruck changed the severity to QA (Quality Assurance)

@MiloTruck
Copy link

This is a good observation, but QA at best.

The only impact here is that an operator will have to unnecessarily wait for 9 days when adding a vault to a DSS, which does not warrant medium severity:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

The function of the protocol of its availability is not impacted, given that vaults can still be added to the DSS.

@c4-judge
Copy link
Contributor

MiloTruck marked the issue as grade-b

@c4-judge c4-judge reopened this Aug 12, 2024
@rbserver rbserver mentioned this issue Aug 15, 2024
@C4-Staff C4-Staff added grade-a and removed grade-b labels Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a insufficient quality report This report is not of sufficient quality QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_20_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

5 participants