stake.link

Summary

The secondary pool allows users to add more SDL tokens to an already removed lock. This could result in funds being indefinitely locked in the pool and an incorrect return value for balanceOf().

Vulnerability Details

In the SDLPoolSecondary contract, all actions must be queued before execution. When a user fully withdraws from a lock, this does not immediately update the owner information, lock details, or reSDL balance. Instead, these updates are queued for later execution.

Simultaneously, if a user deposits more SDL tokens into an existing lock, checks are performed for the current lock owner. However, because the owner and lock information updates are only queued and not carried out as described before, these checks still pass. This allows the user to add more SDL tokens into a lock that has, in fact, been fully withdrawn and will be deleted.

In the end, when both actions are executed in the function _executeQueuedLockUpdates(), the user’s lock is deleted, but totalEffectiveBalance and effectiveBalances[owner] are still increased.

Consequently, the user cannot withdraw or claim rewards from this lock because lockOwners[lockId] has been deleted. Rewards continue being allocated to this lock, but they cannot be claimed.

The PoC is modified from the test should be able to withdraw and burn lock NFT in sdl-pool-secondary.test.ts.

Impact

This issue has three main impacts:

1. SDL tokens are locked, but the lock owner is removed, meaning no one can access or withdraw them. This results in the tokens being locked in the pool forever.

2. Although no one can access the lock, the totalEffectiveBalance still accounts for the amount of locked SDL tokens. This means rewards continue to be distributed to this lock, but no one can claim them, leading to a loss in rewards.

3. The balanceOf() function will return incorrect results when queried.

Recommendations

Actions on a fully withdrawn lock in the secondary pool should be disallowed.