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

Low risk findings compilation

peanuts

[L-01] No upperlimit when setting max locking duration and max boost

There is no upper limit when setting max lock duration and max boost. The owner can set an extremely high amount or an extremely low amount, and the value can be changed any time

function setMaxLockingDuration(uint64 _maxLockingDuration) external onlyOwner {
maxLockingDuration = _maxLockingDuration;
emit SetMaxLockingDuration(_maxLockingDuration);
}
​
function setMaxBoost(uint64 _maxBoost) external onlyOwner {
maxBoost = _maxBoost;
emit SetMaxBoost(_maxBoost);
}

Set an upper limit to both functions, for example, max boost can only be set to 10x. Also, make sure that the value set cannot be too low.

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/LinearBoostController.sol#L45-L59

[L-02] Potential truncation to zero if amount and lockingduration is too low when getting the boost amount

The boost amount depends on four values: amount, maxBoost, _lockingDuration, maxLockingDuration

Since there is no control of the minimum amount to be locked, the calculation may truncate to zero if maxBoost is too high, _lockingDuration is too low and maxLockingDuration is too high.

function getBoostAmount(uint256 _amount, uint64 _lockingDuration) external view returns (uint256) {
if (_lockingDuration > maxLockingDuration) revert MaxLockingDurationExceeded();
> return (_amount * uint256(maxBoost) * uint256(_lockingDuration)) / uint256(maxLockingDuration);
}
​

It also depends on the decimal places of _amount, in case other sdlTokens have lower decimal values.

Consider adding a minimum amount to lock to prevent truncation from happening.

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/LinearBoostController.sol#L36-L40

[L-03] lastLockId has no upper bound, which will cause out of gas errors for certain functions

In function getLockIdsByOwner(), the function loops over every single lock that has been created. If the maxLockId is too large, the function will fail due to out of gas error

function getLockIdsByOwner(address _owner) external view returns (uint256[] memory) {
uint256 maxLockId = lastLockId;
​
> for (uint256 i = 1; i <= maxLockId; ++i) {
if (lockOwners[i] == _owner) {
lockIds[lockIdsFound] = i;
lockIdsFound++;
if (lockIdsFound == lockCount) break;
}
}

Since anyone can create a lock with any amount through _storeNewLock in SDLPoolPrimary, and the lastLockId is always incremented but never decreased, it makes out of gas errors more prominent.

function _storeNewLock(
address _owner,
uint256 _amount,
uint64 _lockingDuration
) internal updateRewards(_owner) {
Lock memory lock = _createLock(_amount, _lockingDuration);
uint256 lockId = lastLockId + 1;
​
locks[lockId] = lock;
lockOwners[lockId] = _owner;
balances[_owner] += 1;
lastLockId++;

Recommend having an upper limit to the lastLockId, and also consider decreasing the max lock count when all the amount is withdrawn. The same issue applies to SDLPoolSecondary, with queuedLocks having no upper bound.

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/base/SDLPool.sol#L177-L187
https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolPrimary.sol#L279-L291

[L-04] Set the base pool as an abstract contract instead of a normal contract

SDLPool is currently a normal contract. Usually, base pools are set to abstract contract, to prevent the base pool from ever being deployed. If the SDLPool is mistakenly deployed, it will cause confusion to both the developers and the users, as there will be another pool address with limited capabilities

contract SDLPool is RewardsPoolController, IERC721Upgradeable, IERC721MetadataUpgradeable {
using SafeERC20Upgradeable for IERC20Upgradeable;

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/base/SDLPool.sol#L14-L17

Updates

Lead Judging Commences

0kage Lead Judge
over 1 year ago
0kage Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

unbounded-locks

getLockIdsByOwner could be very gas intensive and revert

Support

FAQs

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