Summary
Missing min/0 value check for maxBoost and maxLockingDuration variables in constructor can render key functions inoperable and/or effect expected boosting of locked tokens for users e.g. LinearBoostController::getBoostAmount()
QA recommendation included at the end
Vulnerability Details
The LinearBoostController constructor is missing a min/0 value check for the maxBoost and maxLockingDuration variables. These variables can be set in the constructor and their setter functions without a min/0 check. In particular, the getBoostAmount() function doesn’t check the value for 0 which can effect the boost amount for a users’ locked staked tokens (effectively setting to 0 for their locked duration). Additionally, any locked token staking attempt will revert due to the max locking duration exceeding 0; which is required to be above 0 to lock.
constructor(uint64 _maxLockingDuration, uint64 _maxBoost) {
maxLockingDuration = _maxLockingDuration;
maxBoost = _maxBoost;
}
...
function setMaxLockingDuration(uint64 _maxLockingDuration) external onlyOwner {
maxLockingDuration = _maxLockingDuration;
emit SetMaxLockingDuration(_maxLockingDuration);
}
...
function setMaxBoost(uint64 _maxBoost) external onlyOwner {
maxBoost = _maxBoost;
emit SetMaxBoost(_maxBoost);
}
POC
Add to the test/core/sdlPool/sdl-pool-primary.test.ts
toggle commented out code for Test 1 and run in the terminal yarn test
toggle commented out code for Test 2 and run in the terminal yarn test
it.only('setting maxLockingDuration to 0 renders protocol impossible to stake', async () => {
console.log("+ User stakes 100 with lock duration of 365 days, but max boost at 0")
console.log("+ Token balance before stake and lock", await sdlToken.balanceOf(sdlPool.address));
await sdlToken.transferAndCall(
sdlPool.address,
toEther(100),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 365 * DAY])
)
console.log("+ Token balance after stake and lock", await sdlToken.balanceOf(sdlPool.address));
const totalEffBal = fromEther(await sdlPool.totalEffectiveBalance())
const totalStaked = fromEther(await sdlPool.totalStaked())
const lastLockId = (await sdlPool.lastLockId()).toNumber()
const sdlTokenBal = fromEther(await sdlToken.balanceOf(sdlPool.address))
const poolLocks = parseLocks(await sdlPool.getLocks([1]))
console.log("Total effective balance", totalEffBal)
console.log("Total staked", totalStaked)
console.log("Last lock id", lastLockId)
console.log("Sdl token balance", sdlTokenBal)
console.log("Pool locks", poolLocks)
})
Test 1 Results
SDLPoolPrimary
+ Use setMaxBoost() to set boost to 0
+ Max boost is set to: BigNumber { value: "0" }
+ User stakes 100 with lock duration of 365 days, but max boost at 0
+ Token balance before stake and lock BigNumber { value: "0" }
+ Token balance after stake and lock BigNumber { value: "100000000000000000000" }
Total effective balance 100
Total staked 100
Last lock id 1
Sdl token balance 100
Pool locks [
{
amount: 100,
boostAmount: 0,
startTime: 1705001342,
duration: 31536000,
expiry: 0
}
]
✔ setting maxLockingDuration to 0 renders protocol impossible to stake and lock (152ms)
1 passing (7s)
Done in 12.91s.
Test 2 Results
SDLPoolPrimary
+ Max locking duration is set to: BigNumber { value: "0" }
+ User stakes 100 with lock duration of 365 days, but max boost at 0
+ Token balance before stake and lock BigNumber { value: "0" }
1) setting maxLockingDuration to 0 renders protocol impossible to stake and lock
0 passing (7s)
1 failing
1) SDLPoolPrimary
setting maxLockingDuration to 0 renders protocol impossible to stake and lock:
Error: VM Exception while processing transaction: reverted with custom error 'MaxLockingDurationExceeded()'
Impact
Tools Used
Manual review
Recommendations
min or 0 check for constructor and setter functions
Introduce a min boost or 0 value check to the constructor and setMaxBoost() function
Introduce a min duration check to the constructor and the setMaxLockingDuration() function to avoid reverts for user stake and lock, and to improve the UX
constructor(uint64 _maxLockingDuration, uint64 _maxBoost) {
+ if (_maxLockingDuration == 0 || _maxBoost == 0) revert ZeroValue();
if ( _maxLockingDuration
maxLockingDuration = _maxLockingDuration; // @audit no 0 protection
maxBoost = _maxBoost; // @audit no 0 protection
}
...
function setMaxLockingDuration(uint64 _maxLockingDuration) external onlyOwner {
+ if (_maxLockingDuration == 0) revert ZeroValue();
maxLockingDuration = _maxLockingDuration;
emit SetMaxLockingDuration(_maxLockingDuration);
}
...
function setMaxBoost(uint64 _maxBoost) external onlyOwner {
+ if (_maxBoost == 0) revert ZeroValue();
maxBoost = _maxBoost;
emit SetMaxBoost(_maxBoost);
}
Check if var is different from current otherwise return
function setMaxLockingDuration(uint64 _maxLockingDuration) external onlyOwner {
+ if (_maxLockingDuration == maxLockingDuration ) return;
+ if (_maxLockingDuration == 0) revert ZeroValue();
maxLockingDuration = _maxLockingDuration;
emit SetMaxLockingDuration(_maxLockingDuration);
}
...
function setMaxBoost(uint64 _maxBoost) external onlyOwner {
+ if (_maxBoost == maxBoost) return;
+ if (_maxBoost == 0) revert ZeroValue();
maxBoost = _maxBoost;
emit SetMaxBoost(_maxBoost);
}
