A DSS cannot stop staking of a vault that doesn't meet its conditions

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/main/src/entities/Operator.sol#L128

Vulnerability details

Impact

In Operator::validateAndUpdateVaultStakeInDSS() ignoreFailure is set to true which means the DSS cannot cancel a started update even if the vault doesn't meet its conditions anymore.

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
        });

Proof of Concept

Let's look at the following example:

1. A vault is staked to one DSS.
2. Now the operator wants to stake the same vault to another DSS, but that DSS has a condition for accepting new vaults (for example it requires the TVL of the vault to be >= 100 000e18 tokens, assuming 18 decimal precision). The vault currently meets that requirement so the operator calls Core::requestUpdateVaultStakeInDSS() to stake that vault. The DSS accepts it (doesn't revert).
3. There is a 9 days delay until the update can be finalized. That is a lot of time and in that time a lot can happen. For example the first DSS can execute a slashing (it takes 2 days for the slashing to finalize which is way less than the delay of the update) and now totalAssets() of the vault are <= 100 000e18 which no longer meets the second DSS requirement.
4. The operator executes Core::finalizeUpdateVaultStakeInDSS() and even if the DSS reverts, it will not affect the transcation and the state will be updated, so now that vault is staked in both DSSs without the consent of the second one.

Notice that Core::requestUpdateVaultStakeInDSS has an ignoreFailure: !requestStakeUpdate.toStake which is correct, however when finalizing, the ignoreFailure is set to true instead of !toStake. This prevents the DSS from canceling the update if the vault changes somehow in the meantime.

Tools Used

Manual Review

Recommended Mitigation Steps

In Operator::validateAndUpdateVaultStakeInDSS() set ignoreFailure to !queuedStakeUpdate.updateRequest.toStake

    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,
+            ignoreFailure: !queuedStakeUpdate.updateRequest.toStake
            hookCallGasLimit: self.hookCallGasLimit,
            supportsInterfaceGasLimit: self.supportsInterfaceGasLimit,
            hookGasBuffer: self.hookGasBuffer
        });
    }

Assessed type

Invalid Validation

[c4-judge]

MiloTruck marked the issue as unsatisfactory:
Out of scope

[c4-judge]

MiloTruck removed the grade

[c4-judge]

MiloTruck marked the issue as not a duplicate

[c4-judge]

MiloTruck marked the issue as duplicate of #21

[c4-judge]

MiloTruck changed the severity to QA (Quality Assurance)

[c4-judge]

MiloTruck marked the issue as grade-b