The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Low Findings

pronobis4

Summary

Low Risk Issues

Issue Instances
L‑1 Critical functions should be controlled by time locks 19
L‑2 Owner can renounce Ownership 2
L‑3 Missing storage gap for upgradable contracts 1
L‑4 Use Ownable2Step instead of Ownable 1
23

Low Risk Issues

[L‑1] Critical functions should be controlled by time locks

It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user).

Total instances: 19

see instances
// File: contracts/SmartVaultV3.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol
​
114: function liquidate() external onlyVaultManager {
​
135: function removeCollateralNative(uint256 _amount, address payable _to) external onlyOwner {
​
142: function removeCollateral(bytes32 _symbol, uint256 _amount, address _to) external onlyOwner {
​
149: function removeAsset(address _tokenAddr, uint256 _amount, address _to) external onlyOwner {
​
160: function mint(address _to, uint256 _amount) external onlyOwner ifNotLiquidated {
​
214: function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
​
233: function setOwner(address _newOwner) external onlyVaultManager {

GitHub: 114, 135, 142, 149, 160, 214, 233

// File: contracts/SmartVaultManagerV5.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultManagerV5.sol
​
81: function liquidateVault(uint256 _tokenId) external onlyLiquidator {
​
103: function setMintFeeRate(uint256 _rate) external onlyOwner {
​
107: function setBurnFeeRate(uint256 _rate) external onlyOwner {
​
111: function setSwapFeeRate(uint256 _rate) external onlyOwner {
​
115: function setWethAddress(address _weth) external onlyOwner() {
​
119: function setSwapRouter2(address _swapRouter) external onlyOwner() {
​
123: function setNFTMetadataGenerator(address _nftMetadataGenerator) external onlyOwner() {
​
127: function setSmartVaultDeployer(address _smartVaultDeployer) external onlyOwner() {
​
131: function setProtocolAddress(address _protocol) external onlyOwner() {
​
135: function setLiquidatorAddress(address _liquidator) external onlyOwner() {

GitHub: 81, 103, 107, 111, 115, 119, 123, 127, 131, 135

// File: contracts/LiquidationPool.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol
​
182: function distributeFees(uint256 _amount) external onlyManager {

GitHub: 182

// File: contracts/LiquidationPoolManager.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPoolManager.sol
​
84: function setPoolFeePercentage(uint32 _poolFeePercentage) external onlyOwner {

GitHub: 84

[L‑2] Owner can renounce Ownership

Typically, the contract’s owner is the account that deploys the contract. As a result, the owner is able to perform certain privileged activities.

The Openzeppelin’s Ownable used in this project contract implements renounceOwnership. This can represent a certain risk if the ownership is renounced for any other reason than by design. Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.

Total instances: 2

// File: contracts/SmartVaultManagerV5.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultManagerV5.sol
​
16: contract SmartVaultManagerV5 is ISmartVaultManager, ISmartVaultManagerV2, Initializable, ERC721Upgradeable, OwnableUpgradeable {

GitHub: 16

// File: contracts/LiquidationPoolManager.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPoolManager.sol
​
11: contract LiquidationPoolManager is Ownable {

GitHub: 11

[L‑3] Missing storage gap for upgradable contracts

See description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

Total instances: 1

// File: contracts/SmartVaultManagerV5.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultManagerV5.sol
​
16: contract SmartVaultManagerV5 is ISmartVaultManager, ISmartVaultManagerV2, Initializable, ERC721Upgradeable, OwnableUpgradeable {

GitHub: 16

[L‑4] Use Ownable2Step instead of Ownable

Ownable2Step
and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

Total instances: 1

// File: contracts/LiquidationPoolManager.sol
// https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPoolManager.sol
​
11: contract LiquidationPoolManager is Ownable {

GitHub: 11

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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