stake.link

stake.link
DeFiHardhatBridge
27,500 USDC
View results
Submission Details
Severity: medium
Invalid

SDLPoolPrimary::A position can initiate unlocking phase for a stake which was never locked

Summary

An user can stake SDL and initiate unlocking phase without locking it. As a result the user will get rewards.

Vulnerability Details

The problem is in the onTokenTransfer(), in this function there is no check for the lockingDuration so that it is not figured out whether the stake is to be locked or not, it just checks if the lockId is 0 or not, if 0 then it calls _storeNewLock() with that id, as a result the position is locked which is not intended to lock.

POC

Run the test:

it('position can initiate unlocking phase for a stake which was never locked ', async () => {
//@audit
await sdlToken.transferAndCall(
// locked token
sdlPool.address,
toEther(1),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 0])
)
console.log('locked amount: ', parseLocks(await sdlPool.getLocks([1])))
// locked amount: [ { amount: 1, boostAmount: 0, startTime: 0, duration: 0, expiry: 0 } ]
await expect(sdlPool.initiateUnlock(1)).not.to.be.reverted //@audit as [0,0] means still locking i.e one can start unlocking phase
await rewardToken.transferAndCall(sdlPool.address, toEther(1000), '0x')
await time.increase(10 * DAY)
await rewardToken.transferAndCall(sdlPool.address, toEther(1000), '0x')
await time.increase(10 * DAY)
await rewardToken.transferAndCall(sdlPool.address, toEther(1000), '0x')
await time.increase(10 * DAY)
console.log(await rewardsPool.withdrawableRewards(accounts[0]))
// withdrawable rewards: BigNumber { value: "3000000000000000000000" }
console.log('Effective balance: ', fromEther(await sdlPool.effectiveBalanceOf(accounts[0])))
console.log('locked amount: ', parseLocks(await sdlPool.getLocks([1])))
/*
locked amount: [
{
amount: 1,
boostAmount: 0,
startTime: 0,
duration: 0,
expiry: 1703534493 // You can see the expiry time is set
}
]
*/
})

Impact

As shown in POC one can stake very low amount of SDL and his position will automatically be locked and he will able to get rewards. It is an unwanted state for a protocol.

Tools Used

Manual analysis

Recommendations

Put a check in onTokenTransfer() for lockingDuration, if it is set to 0 then don't call _storeNewLock(), instead create a new storage which store all stakes which is not locked & put it into that.

Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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