stake.link

Summary

The SDLPoolPrimary.sol contract exhibits a limitation where users are unable to add additional SDL tokens to an existing lockID without also extending the lock's duration. This behavior is due to a conditional check within the SDLPool.sol::_updateLock function that inappropriately restricts the staking flexibility.

Vulnerability Details

There's a problem in the SDLPoolPrimary.sol contract when User trying to add more tokens to an existing lockID. Specifically, the issue is in two functions: SDLPoolPrimary.sol::_storeUpdatedLock and SDLPool.sol::_updateLock.

When a user tries to increase the stake of an existing lockId by using the SDLPoolPrimary.sol::onTokenTransfer function with a lockingDuration of zero (meaning User don't want to extend the lock's duration), the onTokenTransfer function goes on to call SDLPoolPrimary.sol::_storeUpdatedLock. This then triggers SDLPool.sol::_updateLock, which has a crucial check:

This check is meant to stop the duration of an existing lock from getting shorter. But the problem is, it also stops users from adding funds to their lock if the new lockingDuration is not strictly greater than the existing lock's duration. It doesn't consider the valid scenario where _lockingDuration is zero, indicating the user just wants to add funds without changing the lock's expiry or duration.

So, users are stuck having to extend their lock duration if they want to add more tokens. This limits their ability to manage their stakes flexibly based on their own strategies. This mistake in the contract's logic creates an unnecessary restriction on staking operations, and it could impact how engaged users are and how the staking system works overall.

POC

Step 1: Initial Lock Creation
User have an existing lock (lockId) with an amount of 100 SDL tokens and a locking duration of 1 year.

Step 2: Attempt to Add Additional Funds Without Extending Duration
User attempt to add more SDL tokens to the existing lock by transferring SDL tokens to the contract and specifying the lockId of the existing lock and a lockingDuration of 0 (indicating no desire to extend the duration).

Step 3: SDLPoolPrimary.sol::onTokenTransfer Calls SDLPoolPrimary.sol::_storeUpdatedLock
The onTokenTransfer function is triggered by the token transfer and calls _storeUpdatedLock function, passing User address (_sender), the lockId, the additional amount of SDL tokens, and a lockingDuration of 0.

Step 4: SDLPoolPrimary.sol::_storeUpdatedLock Calls SDLPool.sol::_updateLock
Inside _storeUpdatedLock, the function SDLPool.sol_updateLock is called with the existing lock details, the additional amount, and the lockingDuration of 0.

Step 5: Check in _updateLock Causes Reversion

Since _lockingDuration is 0 and _lock.duration is 1 year, the condition _lockingDuration < _lock.duration is true.
Additionally, since the lock has not expired (_lock.expiry == 0 or _lock.expiry > block.timestamp),The condition _lock.expiry == 0 consistently evaluates to true in cases where the user has not invoked the initiateUnlock function and the entire condition within the if statement is true, causing the function to revert with InvalidLockingDuration().

Impact

Users are forced to extend their lock duration if they wish to increase their staked amount, which may not align with their staking strategy or intentions. This limitation can lead to user dissatisfaction and reduced participation in the staking mechanism, potentially impacting the total value locked (TVL) in the protocol.

Tools Used

Manual Review

Recommendations

The contract should be updated to allow users to add funds to an existing lock without requiring an increase in the locking duration.