pauseForceAllVaults(), unpauseForceAllVaults(), pauseActionInAllVaults(), and upauseActionInAllVaults() function used to pause and unpause the vaults. this function internally calls _changePauseState() function to execute action.
_changePauseState() method loops over all the vaults to change their pause state. If there are a large number of vaults, this method may exceed the gas limit and fail to execute.
as similar to this setSafetyRating() is internally used _checkValidVault() function and it also loop over the _vaults. as mentioned above setSafeRating() is also will not able to execute. if setSafetyRating() will not work then deployVault() will not work because it depends on it.
in future if there is a bug and there is a need of pause the contract, if _vaults array will be large enough then it cannot pause the contracts and it can be harmful for the protocol because there is no method for the pause vaults individually as well.
Attack scenario
deployVault() can be able to deploy and push vaults
1.permissionlessDeployments is there to deploy the vaults by anyone. if permissionlessDeployments = true
then attacker can deploy as many vaults as it can stop the other functionalities as mentioned in the description.
Recommendation
add limit to _vaults and add functionality to pause _vaults individually.
0xdcota
commented
1 year ago
Severity
Medium
Affected Smart Contracts
Chief.sol
Description
pauseForceAllVaults(),unpauseForceAllVaults(),pauseActionInAllVaults(), andupauseActionInAllVaults()function used to pause and unpause the vaults. this function internally calls_changePauseState()function to execute action.https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/Chief.sol#L367-L375
_changePauseState()method loops over all the vaults to change their pause state. If there are a large number of vaults, this method may exceed the gas limit and fail to execute.as similar to this
setSafetyRating()is internally used_checkValidVault()function and it also loop over the _vaults. as mentioned abovesetSafeRating()is also will not able to execute. ifsetSafetyRating()will not work then deployVault() will not work because it depends on it.https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/Chief.sol#L404-L419
Impact
in future if there is a bug and there is a need of pause the contract, if _vaults array will be large enough then it cannot pause the contracts and it can be harmful for the protocol because there is no method for the pause vaults individually as well.
Attack scenario
deployVault()can be able to deploy and push vaults 1.permissionlessDeploymentsis there to deploy the vaults by anyone. ifpermissionlessDeployments = trueRecommendation
add limit to _vaults and add functionality to pause _vaults individually.