stake.link
stake.link
DeFiHardhatBridge
27,500 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Additional tokens can still be locked into a `lockId` that is going to be erased

naruto

Summary

When the amount of locked tokens is fully removed from SDLPoolPrimary, the associated lockId is immediately erased. However, even if all tokens have been withdrawn from SDLPoolSecondary, the lockId will persist and will be destroyed only when the owner of that lockId calls the executeQueuedOperations() function. Users can still lock additional tokens into the lockId as long as it exists. Users will lose all locked tokens if the order to withdraw all tokens within the lockId comes before any further lock orders for the same lockId.

Vulnerability Details

Vulnerability Steps:

  • The user initially locks a quantity of tokens in SDLPoolSecondary, and the lockId exists.

  • The user performs an init and withdraws all tokens within the aforementioned lockId. The lockId is not destroyed at this point, and the withdraw order is added to queuedLockUpdates with lock.amount set to 0.

  • The user notices that the lockId still exists and locks additional tokens into it. The additional lock order is added to queuedLockUpdates.

  • Now, queuedLockUpdates contains numerous orders, with the withdraw order at the front of the queue.

  • Finally, the _executeQueuedLockUpdates() function is called. The lockId falls into the withdrawal case and gets deleted (L471-L473).

  • lockOwners[lockId] is deleted, additional locks can still be added to that lockId after that, but they will not be owned by anyone and will be permanently stuck within.

Test

it('POC', async () => {
await mintLock()
await time.increase(200 * DAY)
await sdlPool.initiateUnlock(1)
await time.increase(200 * DAY)
​
await updateLocks()
​
await sdlPool.withdraw(1, toEther(100))
​
let newAmount = parseLockUpdates(await sdlPool.getQueuedLockUpdates([1]))[0][0].lock.amount
assert.equal(newAmount, 0)
assert.equal(await sdlPool.ownerOf(1), await signers[0].getAddress())
​
await sdlToken.transferAndCall(
sdlPool.address,
toEther(100),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [1, 365 * DAY])
)
newAmount = parseLockUpdates(await sdlPool.getQueuedLockUpdates([1]))[0][1].lock.amount
assert.equal(newAmount, 100)
assert.equal(await sdlPool.ownerOf(1), await signers[0].getAddress())
await updateLocks()
console.log(parseLocks(await sdlPool.getLocks([1]))) // To use this command, you must comment out line 165 in the SDLPool contract.
await expect(sdlPool.ownerOf(1)).to.be.revertedWith('InvalidLockId()')
})

Impact

  • This confusion could lead to the permanent loss of tokens that users lock additionally.

Recommendations

  • Do not add any orders with an existing lockId that has lock.amount = 0 until it is deleted, or

  • After completing the iteration through the queuedLockUpdates loops, then perform the check and deletion.

Updates

Lead Judging Commences

0kage Lead Judge
almost 2 years ago
0kage Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

add-to-old-lock

User trying to update a fully withdrawn lock in same batch id on secondary pool

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!