Summary
When a staker calls the extend extendLockDuration function to extend their lock duration, the initial boostAmount get overwritten by the new boostAmount, causing the user to lose boostAmount for the initial period that his stakes were locked.
This also happens when they increase lock amount, the boostAmount with the previous amount and duration get overwritten.
The root cause of this Bug comes from the _updateLock function.
Vulnerability Details
SDLPool.sol: calcutates boostAmount as
Lock memory lock = Lock(_lock.amount, _lock.boostAmount, _lock.startTime, _lock.duration, _lock.expiry);
uint256 baseAmount = _lock.amount + _amount;
uint256 boostAmount = boostController.getBoostAmount(baseAmount, _lockingDuration);
This cause a problem as the boostAmount for the initial lockperiod get overwritten with the new boostAmount for Example.
Suppost initial lock duration was 90days
Then after 60days, the staker decides to extend lock duration by 120days, making the total lock duration to be 60days + 120day = 180days.
The protocol calculates the boostAmount for 120days and ignores the 60days that have been already elapsed making the staker to lose 60days worth of boostAmount
Impact
Updating Lock leads to lose in profit for staker.
POC
Add this test to this file and run the following command
yarn test ./test/core/sdlPool/sdl-pool-primary.test.ts --grep "POC"
describe("POC",async () => {
it('Updating Lock Durations', async () => {
await sdlToken.transferAndCall(
sdlPool.address,
toEther(100),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 0])
)
await sdlPool.extendLockDuration(1, 90 * DAY)
let ts = (await ethers.provider.getBlock(await ethers.provider.getBlockNumber())).timestamp
const maxLockDuration = (365 * 4) * DAY
const boostAmountInitial = Number((100 * 90 * 4 * DAY / maxLockDuration).toFixed(4))
assert.equal(fromEther(await sdlPool.totalEffectiveBalance()), 124.65753424657534)
assert.equal(fromEther(await sdlPool.totalStaked()), 124.65753424657534)
assert.equal(fromEther(await sdlPool.effectiveBalanceOf(accounts[0])), 124.65753424657534)
assert.equal(fromEther(await sdlPool.staked(accounts[0])), 124.65753424657534)
assert.deepEqual(parseLocks(await sdlPool.getLocks([1])), [
{ amount: 100, boostAmount: boostAmountInitial, startTime: ts, duration: 90 * DAY, expiry: 0 },
])
await time.increase(60 * DAY)
const boostAmountFinal = Number((100 * 180 * 4 * DAY / maxLockDuration).toFixed(4))
await sdlPool.extendLockDuration(1, 120 * DAY)
ts = (await ethers.provider.getBlock(await ethers.provider.getBlockNumber())).timestamp
assert.deepEqual(parseLocks(await sdlPool.getLocks([1])), [
{ amount: 100, boostAmount: boostAmountFinal , startTime: ts, duration: 120 * DAY, expiry: 0 },
])
})
})
Tools Used
Manual Analysis and Hardhat
Recommendations
Added the following codes snippet to the _updateLock function
function _updateLock(
Lock memory _lock,
uint256 _amount,
uint64 _lockingDuration
) internal view returns (Lock memory) {
if ((_lock.expiry == 0 || _lock.expiry > block.timestamp) && _lockingDuration < _lock.duration) {
revert InvalidLockingDuration();
}
Lock memory lock = Lock(_lock.amount, _lock.boostAmount, _lock.startTime, _lock.duration, _lock.expiry);
uint256 baseAmount = _lock.amount + _amount;
uint256 boostAmount = boostController.getBoostAmount(baseAmount, _lockingDuration);
+ if (lock.duration > 0) {
+ boostAmount = boostAmount + (lock.boostAmount * (uint64(block.timestamp) - lock.startTime) / lock.duration);
+ }
if (_lockingDuration != 0) {
lock.startTime = uint64(block.timestamp);
} else {
delete lock.startTime;
}
lock.amount = baseAmount;
lock.boostAmount = boostAmount;
lock.duration = _lockingDuration;
lock.expiry = 0;
return lock;
}
