INPUT VALIDATION SHOULD BE PERFORMED FOR THE _cycleParams PASSED INTO THE registerProtectionPool() FUNCTION
Summary
In order to register a protection pool registerProtectionPool() function is called with _cycleParams as one of the input parameters in the ProtectionPoolCycleManager.sol contract. But the input value of _cycleParams is not checked against the 0 value.
Vulnerability Detail
The _cycleParams is not checked for the 0 value inside the function. Hence the 0 can be passed in as both the _cycleParams.openCycleDuration and _cycleParams.cycleDuration value. The function checks for the _cycleParams.openCycleDuration > _cycleParams.cycleDuration condition. But if 0 is passed in for both parameters the condition will pass and transaction will continue. Hence there is no explicit check for the 0 value of passed in for _cycleParams.openCycleDuration and _cycleParams.cycleDuration values.
Impact
If the 0 value is passed in for _cycleParams.openCycleDuration and _cycleParams.cycleDuration values via the passed in _cycleParams parameter, same 0 values will be assigned as the poolCycle.params for the new cycle started. So this will create a cycle index without any duration which will not make any sense. Thus making it impossible to withdraw underlying tokens from the protection pools. Hence the underlying tokens of the depositors could be locked for that particular cycle index. Hence new request for withdrawal will need to be submitted to withdraw their underlying tokens in a future cycle index.
Code Snippet
function registerProtectionPool(
address _poolAddress,
ProtectionPoolCycleParams calldata _cycleParams
) external payable override onlyContractFactory {
ProtectionPoolCycle storage poolCycle = protectionPoolCycles[_poolAddress];
if (poolCycle.currentCycleStartTime > 0) {
revert ProtectionPoolAlreadyRegistered(_poolAddress);
}
/// Cycle duration must be greater than open cycle duration
if (_cycleParams.openCycleDuration > _cycleParams.cycleDuration) {
revert InvalidCycleDuration(_cycleParams.cycleDuration);
}
/// Store cycle params and start a new cycle for newly registered pool
poolCycle.params = _cycleParams;
_startNewCycle(_poolAddress, poolCycle, 0);
}
Udsen
high
INPUT VALIDATION SHOULD BE PERFORMED FOR THE
_cycleParamsPASSED INTO THEregisterProtectionPool()FUNCTIONSummary
In order to register a protection pool
registerProtectionPool()function is called with_cycleParamsas one of the input parameters in theProtectionPoolCycleManager.solcontract. But the input value of_cycleParamsis not checked against the0value.Vulnerability Detail
The
_cycleParamsis not checked for the0value inside the function. Hence the0can be passed in as both the_cycleParams.openCycleDurationand_cycleParams.cycleDurationvalue. The function checks for the_cycleParams.openCycleDuration > _cycleParams.cycleDurationcondition. But if0is passed in for both parameters the condition will pass and transaction will continue. Hence there is no explicit check for the0value of passed in for_cycleParams.openCycleDurationand_cycleParams.cycleDurationvalues.Impact
If the
0value is passed in for_cycleParams.openCycleDurationand_cycleParams.cycleDurationvalues via the passed in_cycleParamsparameter, same0values will be assigned as thepoolCycle.paramsfor the new cycle started. So this will create a cycle index without any duration which will not make any sense. Thus making it impossible to withdraw underlying tokens from the protection pools. Hence the underlying tokens of the depositors could be locked for that particular cycle index. Hence new request for withdrawal will need to be submitted to withdraw their underlying tokens in a future cycle index.Code Snippet
https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/ProtectionPoolCycleManager.sol#L73-L91
Tool used
VS Code and Manual Review
Recommendation
Conduct the value check for
0value for both_cycleParams.openCycleDurationand_cycleParams.cycleDuration, before the new cycle is started.Add following code snippet to the
registerProtectionPool()function by creating a new errorZeroCycleDuration().