DSS can deposit assets into the vault by oneself, which can lead to vault users losing more slashed assets

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/main/src/Core.sol#L220-L231

Vulnerability details

Impact

DSS can deposit assets into the vault by oneself, which can lead to vault users losing more slashed assets.

Proof of Concept

The protocol implements two distinct time windows to manage asset withdrawals and slashing:

• Users have a 9-day period to redeem assets from the vault.
• DSS has a 2-day window to execute slashing on the vault.

These time windows are designed to prevent users from circumventing slashing penalties by withdrawing assets prematurely.

However, a potential vulnerability exists in this mechanism:
DSS, having control over the slashing timestamp and the ability to deposit assets into the vault, could exploit this system. The exploit would proceed as follows:

• DSS deposits a significant amount of assets into the target vault.
• DSS initiates the slashing process, triggering the 2-day slashing window.

Just before calling finalizeSlashing at the end of the 2-day window, DSS withdraws the previously deposited assets.

test:

function testDSSCanSlashMoreAssets() public {
    address alice = makeAddr('alice');
    address bob = makeAddr('bob');
    address dssUser = makeAddr('dssUser');
    address dssContract = address(this);

    core.registerDSS(10e18);
    vm.prank(operator);
    core.registerOperatorToDSS(IDSS(dssContract),'');

    depositToken.mint(alice, 1200);
    depositToken.mint(bob, 1200);
    depositToken.mint(dssUser, 1200);

    vm.startPrank(alice);
    depositToken.approve(address(vault), 1200);
    vault.deposit(1200, alice);
    vm.stopPrank();

    vm.startPrank(bob);
    depositToken.approve(address(vault), 1200);
    vault.deposit(1200, bob);
    vm.stopPrank();

    vm.startPrank(dssUser);
    depositToken.approve(address(vault), type(uint256).max);
    vault.approve(address(vault), type(uint256).max);
    vault.deposit(1200, dssUser);
    vm.stopPrank();

    //register vault to dss.
    Operator.StakeUpdateRequest memory stakeVault = Operator.StakeUpdateRequest({
        vault:address(vault),
        dss:IDSS(dssContract),
        toStake:true
    });
    vm.prank(operator);
    Operator.QueuedStakeUpdate memory updatedStake = core.requestUpdateVaultStakeInDSS(stakeVault);
    skip(9 days + 1);

    core.finalizeUpdateVaultStakeInDSS(updatedStake);

    //dss startRedeem.
    vm.startPrank(dssUser);
    bytes32 withdrawalKey = vault.startRedeem(vault.balanceOf(dssUser), dssUser);
    vm.stopPrank();

    skip(7 days + 1);
    
    //dss request slash specific vault.
    uint96[] memory percentages = new uint96[](1);
    address[] memory vaultAddresses = new address[](1);
    percentages[0] = 10e18; //10% percent.
    vaultAddresses[0] = address(vault);
    SlasherLib.SlashRequest memory slashingRequest = SlasherLib.SlashRequest({
        operator: operator,
        slashPercentagesWad: percentages,
        vaults: vaultAddresses
    });
    SlasherLib.QueuedSlashing memory queuedSlashing = core.requestSlashing(slashingRequest);

    skip(2 days);
    //request slash passed after 2 day.

    //dss finishRedeem
    vm.prank(dssUser);
    vault.finishRedeem(withdrawalKey);

    //dss user get back assets.
    assertEq(1200,depositToken.balanceOf(dssUser));
    
    //finalizeSlashing
    // core.finalizeSlashing(queuedSlashing);
}

From above test we can see dssUser withdraw all assets from vault, and leave all slash to remaining vault user.

Tools Used

Foundry

Recommended Mitigation Steps

I don't have a good solution to mitigate the aforementioned issue. However, I am certain that DSS can cause vault users to lose more assets through the process described above。

Assessed type

Other

[c4-judge]

MiloTruck marked the issue as not a duplicate

[MiloTruck]

Invalid.

The DSS is assumed to be non-malicious - operators will only register and deploy vaults to a DSS that it trusts.

[c4-judge]

MiloTruck marked the issue as unsatisfactory:
Invalid

[c4-judge]

MiloTruck marked the issue as unsatisfactory:
Invalid

[coffiasd]

@MiloTruck

according to audit page we can see DSS is not a trusted role
Sure, here’s the translation:

Moreover, the most important thing is that finishRedeem and finalizeSlashing can be executed within the same transaction. This means that the so-called Slashing Veto Committee has no chance to detect the issue because, before the slash occurs, the DSS shares are still in the vault.

28

[MiloTruck]

The DSS is not marked as trusted role, but it is an implicit assumption as operators have to manually register themselves to a DSS. They should be doing due diligence to ensure the DSS isn't malicious in the first place.

Also, from the sponsor:

This is kinda farfetched as if a malicious DSS just deposits and queue a withdrawal it's potentially exposing itself to get slashed by other DSSs too.

[TradMod]

The DSS is not marked as trusted role, but it is an implicit assumption as operators have to manually register themselves to a DSS. They should be doing due diligence to ensure the DSS isn't malicious in the first place.

You are correct sir but the thing is, a DSS can behave maliciously in future maybe when the operator tries to Unstake from that DSS.
Kinda Same thing that is happening in the #55, In that issue it should be DSS responsibility to check the operator's given slashingHandler but it is got accepted, and I totally understand why, because the code allows that to happen. Same is the case here, the code allows the DSS to turn malicious and slash 100% of the assets of an operator.

Also, from the sponsor:
This is kinda farfetched as if a malicious DSS just deposits and queue a withdrawal it's potentially exposing itself to get slashed by other DSSs too.

This is correct as well but from my understanding an operator can have only one DSS.

Operators with one DSS will be most open to these attacks.