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

QA-report

rafty
ID Description Severity
L-01 Owner can renounce ownership. Low
L-02 No boundaries when setting values. Low
L-03 Errors not used. Low
L-04 Initialization can be frontrun. Low
L-05 Do not use assert. Low
L-06 Missing array length check. Low

[L-01] Owner can renounce ownership.

Description

LinearBoostController.sol is inherited from Ownable contract, therefore it's possible for owner to use renounceOwnership function and left the ownership.

contract LinearBoostController is Ownable {

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

In RewardsInitiator.sol:

contract RewardsInitiator is Ownable {

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/RewardsInitiator.sol#L16

In RESDLTokenBridge.sol:

contract RESDLTokenBridge is Ownable {

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/RESDLTokenBridge.sol#L16

In SDLPoolCCIPController.sol:

abstract contract SDLPoolCCIPController is Ownable, CCIPReceiver {

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/base/SDLPoolCCIPController.sol#L13-L13C67

Recommendation

Cosnider to override function and revert when owner try to call it.

++ function renounceOwnership() public override onlyOwner {
++ revert OperationNotAllowed();
++ }

[L-02] No boundaries when setting values.

Description

There are some functions where variables can be set to any value due to there is no restriction to min/max bounds.

In LinearBoostController.sol:

maxLockingDuration = _maxLockingDuration;
maxBoost = _maxBoost;

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

maxLockingDuration = _maxLockingDuration;

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

maxBoost = _maxBoost;

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

In SDLPoolCCIPController.sol

function setMaxLINKFee(uint256 _maxLINKFee) external onlyOwner {
maxLINKFee = _maxLINKFee;
}

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/base/SDLPoolCCIPController.sol#L130-L132

Recommendations

Consider to add min/max bounds to these funtions.

[L-03] Errors not used.

Description

In SDLPool.sol contracts some errors not used:

error InvalidParams();

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

error DuplicateContract();
error ContractNotFound();

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

In SDLPoolCCIPController.sol

error FeeExceedsLimit(uint256 fee);

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/base/SDLPoolCCIPController.sol#L30

Recommendations

Consider remove or use them.

[L-04] Initialization can be frontrun.

Description

__SDLPoolBase_init function can be used by anyone, so it's possible for malicious user to frontrun contract initialization.

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

In SDLPoolPrimary.sol contract initialize function also missing modifier and can be used by anyone.

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

In SDLPoolSecondary.sol contract initialize function missing modifier

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

Recommendations

Add restriction so only the owner can use initialize function.

[L-05] Do not use assert.

Description

In versions of Solidity prior to 0.8.0, when encountering an assert all the remaining gas will be consumed. Even after solidity 0.8.0, the assert function is still not recommended, as described in the documentation.

assert(lockIdsFound == lockCount);

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

Recommendations

Consider to use require or revert with custom errors instead of assert.

[L-06] Missing array length check.

Description

There is not check that array length in not equal to zero in checkUpkeep function.

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/RewardsInitiator.sol#L49

Recommendations

Add check that array length in not equal to zero as in performUpkeep function.

function checkUpkeep(bytes calldata) external view returns (bool, bytes memory) {
address[] memory strategies = stakingPool.getStrategies();
++ if (strategies.length == 0) revert NoStrategiesToUpdate();
​
bool[] memory strategiesToUpdate = new bool[](strategies.length);
...
}
Updates

Lead Judging Commences

0kage Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

renounce

accidentally renouncing ownership

Support

FAQs

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