stake.link

Summary

The owner can withdraw the full amount from the withdraw function.
Suppose that after a reset, a staking is attempted again for the same lockId.
The process is queued and the funds are successfully added, but a lock without an owner is created during the execution phase and the owner cannot withdraw the funds.
In the primary chain, there is no problem because both the lock and ownership are removed as soon as the full amount is withdrawn. This occurs only in the secondary chain.

Vulnerability Details

In the withdraw function of SDLPoolSecondary.sol, the amount can be withdrawn in full. However, at this stage, the lock's ownership rights are not yet removed.

    lock.amount = baseAmount - _amount;
    effectiveBalances[msg.sender] -= _amount;
    totalEffectiveBalance -= _amount;
    queuedLockUpdates[_lockId].push(LockUpdate(updateBatchIndex, lock));

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolSecondary.sol#L230-L234

In the _queueLockUpdate function, funds can be added freely thereafter.
It is standard behavior for users to reset the condition once and then create a lock with new conditions.

    Lock memory lock = _getQueuedLockState(_lockId);
    LockUpdate memory lockUpdate = LockUpdate(updateBatchIndex, _updateLock(lock, _amount, _lockingDuration));
    queuedLockUpdates[_lockId].push(lockUpdate);

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolSecondary.sol#L434-L436

In _executeQueuedLockUpdates, if the full amount is withdrawn, the lock and its owner are deleted.

                if (updateLockState.amount == 0) {
                    delete locks[lockId];
                    delete lockOwners[lockId];

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolSecondary.sol#L470-L472

The process continues and another new lock (same Id) is created when funds are added.
However. The owner of the lock is not added at this time.

                locks[lockId] = updateLockState;
                uint256 totalDiff = uint256(baseAmountDiff + boostAmountDiff);
                effectiveBalances[_owner] += totalDiff;

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolSecondary.sol#L485-L487

All actions allowed to the owner of the lock, including withdraw, will not be allowed.
So, no one can withdraw funds.

This problem does not occur in the primary chain because the lock is also deleted the moment the full amount is withdrawn. (No additional actions can be performed on the same Id).
This can happen only in the secondary chain, and the inconsistency of its behavior is also a problem.

Impact

Users lose funds.

Tools Used

Manual

Recommendations

After the full withdrawal, no additional actions should be allowed (not in the execute phase, and no additional actions should be allowed to even be put into queue).