Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/registries/contracts/staking/StakingBase.sol#L818-L820

Vulnerability details

Relevant code: StakingBase::unstake

Description

The Staking contract is designed to make sure a service can be unstaked only under certain conditions. This is done to prevent exploitative actions like staking and unstaking as soon as possible after rewards are distributed.

Inside the unstake function, one such preventative check ensures a service can't be unstaked before the minimum duration, when the Staking contract has rewards:

if (ts <= minStakingDuration && availableRewards > 0) {
    revert NotEnoughTimeStaked(serviceId, ts, minStakingDuration);
}

This mechanism can be gamed by malicious actors who can deposit dust amounts in the Staking contract and unfairly prevent services from unstaking. This has the unintended consequence of keeping services in an essentially empty Staking contract.

Deposit functions in StakingToken and StakingNativeToken respectively

function deposit(uint256 amount) external {
    uint256 newBalance = balance + amount;
    uint256 newAvailableRewards = availableRewards + amount;
    ...
}

receive() external payable {
    uint256 newBalance = balance + msg.value;
    uint256 newAvailableRewards = availableRewards + msg.value;
    ...
}

Have no checks for minimum deposit amount, making this attack really cheap. Because of the comparison to 0, availableRewards can be as small as 1 wei.

Root Cause

Dust rewards prevent unstaking before minimum duration has passed.

Impact

A service can be kept in an unfavourable Staking contract. This causes opportunity loss for the owner, because they could stake in another contract during this time. Service owners can keep the competition locked in another staking contract.

PoC

Imagine the following scenario:

5 services are in a Staking contract with long min duration.
A 6th one joins.
Another Staking contract is instantiated with more favourable rewards.
All 5 services unstake without any issues.
Just after that, someone deposits dust amounts in the Staking contract to reduce the competition in the newer Staking contract.
Service #6 now has to either - wait for the next checkpoint, collect rewards and try to unstake, or wait the whole min stake duration and then unstake.

In the worst case scenario, multiple services are left in the Staking contract and a malicious actor deposits dust after each checkpoint() call, forcing them to wait the minimum stake duration.

Suggested Mitigation

Unfortunately, the only way to completely prevent abusing this mechanism is to remove the availableRewards > 0 check and make all services wait for the minimum duration. This would not be ideal, but at least the opportunity loss potential would be equal to all.

A more precise solution would be to add an additional StakingParam that would control the right-hand side of the check:

if (ts <= minStakingDuration && availableRewards > minStakingFunds) { ... }

In this way, such an attack can be a lot more costlier and will disincentivize it.

It would also help to prevent deposit of dust amounts in the corresponding Staking contracts.

Assessed type

DoS

code-423n4 / 2024-05-olas-validation

Dusting Staking contract can prevent unstaking #252