Request update stake can be repeated for a vault to a DSS even when the vault is staked already

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

The vault when staked newly to a DSS, invokes certain code from the DSS to support initialization. The vulnerability allows existing staked vaults to be staked again which might reinitialize or corrupt the states of the existing vault configuration for the DSS.

Proof of Concept

In the validateAndUpdateVaultStakeInDSS function from finalizeUpdateVaultStakeInDSS in Core, we see that we update the vault stake in the DSS.

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

In updateVaultStakeInDSS, We see that the enumerableSet library's return value isn't checked. There is also no .contains() method used to check if the .add and .remove in this code segment is valid.

    function updateVaultStakeInDSS(State storage operatorState, address vault, IDSS dss, bool toStake) internal {
        if (toStake) {
            operatorState.vaultStakedInDssMap[dss].add(vault);
        //@audit, it isn't checked if the vault exists already in the dssMap
        } else {
            operatorState.vaultStakedInDssMap[dss].remove(vault);
        }
    }

This allows requestUpdateVaultStakeInDSS to be called once again after the first one is finalized for the same vault and the corresponding call to the DSS executed, despite the vault being staked already for the DSS.

Tools Used

Manual analysis

Recommended Mitigation Steps

Check if the dssMap contains the vault already, and if yes revert.

Assessed type

Invalid Validation

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Out of scope

[c4-judge]

MiloTruck marked the issue as not a duplicate

[c4-judge]

MiloTruck removed the grade

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Invalid

[c4-judge]

MiloTruck removed the grade

[c4-judge]

MiloTruck marked the issue as primary issue

[c4-judge]

MiloTruck changed the severity to QA (Quality Assurance)

[MiloTruck]

The warden has not demonstrated how this issue has an impact that meets the requirements for high/medium severity.

The vulnerability allows existing staked vaults to be staked again which might reinitialize or corrupt the states of the existing vault configuration for the DSS.

The impact highlighted here depends on the DSS implementation, which is not in-scope. You could also argue that it is the responsibility of the DSS to handle repeated calls to finalizeUpdateVaultStakeInDSS().

Therefore, this issue is QA at best.

[c4-judge]

MiloTruck marked the issue as grade-b

[devdks25]

Fix: https://github.com/karak-network/karak-restaking/pull/364