stake.link

Summary

In the secondary pool, a user can mistakenly update the same lockId he already fully withdrew (and the full withdrawal is on the queue) by staking more on it. Once the updates are executed, the staked amount will be permanently locked in the SDLPoolSecondary contract without the possibility of withdrawal because the ownership of the lockId is removed during the execution of the first full withdrawal update.

Vulnerability Details

On the Secondary Pool, when a user performs any action that will alter their effective reSDL balance such as staking, locking, withdrawing, or initiating a withdrawal, the action will be queued instead of taking effect immediately. A user can perform as many updates as he wants on a given lock during a batch. Once an update comes from the primary pool, the queued updates will be executed in their requested order, and if a lock has several updates that are not dispatched in the correct order, a state disaster can occur.

Consider the following series of updates on a given lockId 1 that belongs to Bob in which after the execution of updates there will be a permanent lock of SDLToken on the Secondary Pool contract with ownership loss of the lock:

• Bob is the owner of a lockId 1 with the following information: Lock(amount:100, boostAmount:0, startTime: 0, duration: 0, expiry: 0)

• Bob withdraws all his staked amount in lockId 1, the action will be queued and pushed to queuedLockUpdates

• Bob stakes on the same lockId 1 by transferring 200 SDL tokens, so onTokenTransfer is called and the _calldata passed will be (1, 0) (lockId 1 with 0 lockingDuration), the update is queued by pushing it to queuedLockUpdates

Suppose now the CCIPController sends the update to the primary chain and the updates are confirmed:

• Bob calls the function executeQueuedOperations to execute his pending operations

• The internal function _executeQueuedLockUpdates will be executed, it will loop through the pending updates of the lockId 1:

  • The first update is a full withdrawal of the staked amount in lockId 1, so this block will be executed. Notice that delete lockOwners[lockId]; and balances[_owner] -= 1; will be executed and the ownership is removed from lockId 1

  • The second queued update gets executed in the second iteration of the loop, and as the baseAmountDiff is greater than 0, this block will be executed. The locks[lockId] gets updated to the new lock state, the effectiveBalance of Bob gets increased by 200, and the totalEffectiveBalance gets increased by 200 BUT the lockOwners[1] is still deleted in the first update and is not handled in the second update

After the execution of updates, locks[1] will contain valid stake information (locks[1] = Lock(amount:200, boostAmount:0, startTime: 0, duration: 0, expiry: 0)) but with no owner associated to it as lockOwners[1] is deleted in the first update, so the stakes are locked in the contract and Bob can not withdraw his stakes as he is no longer the owner of lockId 1. The pool has no mechanism to recover the permanent locked SDL.

PoC

Impact

This issue will have the following impacts:

• Permanent lock of stakes on the secondary pool without a possiblity of recovery.

• Users may lose control on their stakes

Tools Used

Manual Review

Recommendations

Consider preventing users from updating a lock if there's already a full withdrawal of the lock on the queue. Below is a suggestion code to fix the issue: