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

SDLPoolPrimary: update lock would undermine the locked amount with boost in some cases

ak1

Summary

_storeUpdatedLock function would undermine the old locked value. due this , effective locked value is undermined which is not favorable to the end user.

Vulnerability Details

The locking mechanism works in such a manner, where,

  1. user would create a lock. Boost amount is calculated based on the boost value and max lock time.

  2. after creating the lock, the can update the time or locked amount.

the boost value which is used to calculate the boost amount can be adjusted by the owner.

Lets look at the _storeUpdatedLock which us used in above cases.

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolPrimary.sol#L307-L329

function _storeUpdatedLock(
address _owner,
uint256 _lockId,
uint256 _amount,
uint64 _lockingDuration
) internal onlyLockOwner(_lockId, _owner) updateRewards(_owner) {
Lock memory lock = _updateLock(locks[_lockId], _amount, _lockingDuration);
​
int256 diffTotalAmount = int256(lock.amount + lock.boostAmount) -
int256(locks[_lockId].amount + locks[_lockId].boostAmount); -------------------------->>> update lock would be higher.
​
if (diffTotalAmount > 0) {
effectiveBalances[_owner] += uint256(diffTotalAmount);
totalEffectiveBalance += uint256(diffTotalAmount);
} else if (diffTotalAmount < 0) { -------------------------------->>>>>>>>>>> this would undermine users lock value.
effectiveBalances[_owner] -= uint256(-1 * diffTotalAmount);
totalEffectiveBalance -= uint256(-1 * diffTotalAmount);
}
​
locks[_lockId] = lock;
​
emit UpdateLock(_owner, _lockId, lock.amount, lock.boostAmount, lock.duration);
}

as shown above, when updating the lock, the old lock and boost values are deducted from the updated lock and boost value. This would result in negative value when the boost value are changed.

This will lead to execution of else block and reduction of the effective balance of the user.

Impact

User's lock values are reduced. They loss their locked share value.

Tools Used

Manual review.

Recommendations

Do not allow reduction of amount. update the code as shown below.
https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolPrimary.sol#L307-L329

function _storeUpdatedLock(
address _owner,
uint256 _lockId,
uint256 _amount,
uint64 _lockingDuration
) internal onlyLockOwner(_lockId, _owner) updateRewards(_owner) {
Lock memory lock = _updateLock(locks[_lockId], _amount, _lockingDuration);
​
int256 diffTotalAmount = int256(lock.amount + lock.boostAmount) -
int256(locks[_lockId].amount + locks[_lockId].boostAmount);
​
if (diffTotalAmount > 0) {
effectiveBalances[_owner] += uint256(diffTotalAmount);
totalEffectiveBalance += uint256(diffTotalAmount);
} else if (diffTotalAmount < 0) { ------------------------------------>>> remove this else block.
effectiveBalances[_owner] -= uint256(-1 * diffTotalAmount);
totalEffectiveBalance -= uint256(-1 * diffTotalAmount);
}
​
locks[_lockId] = lock;
​
emit UpdateLock(_owner, _lockId, lock.amount, lock.boostAmount, lock.duration);
}
Updates

Lead Judging Commences

0kage Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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