However, it is incorrect and leads to an incorrect calculation of the length of the _totalRewardsClaimable array.
The _totalRewardsClaimable array is used to store the CVX rewards for each ERC20 token across multiple cycles. The size of this array should be equal to the number of cycles between the next claimable cycle (nextClaimableCvx) and the current cycle (actualCycle), inclusive.
However, the current line uint256 maxLengthRewards = nextClaimableCvx; sets the length of the _totalRewardsClaimable array to be equal to nextClaimableCvx. This is incorrect because the function needs to calculate and store CVX rewards for multiple cycles, not just the nextClaimableCvx cycle.
For example:
Let's say nextClaimableCvx is 10, and actualCycle is 15. This means that the function needs to calculate and store CVX rewards for 6 cycles (10, 11, 12, 13, 14, and 15).
With the current line uint256 maxLengthRewards = nextClaimableCvx;, maxLengthRewards will be set to 10. This means that the _totalRewardsClaimable array will have a length of 10, which is not enough to store the CVX rewards for all 6 cycles.
As a result, when the function tries to store the CVX rewards for cycles 11, 12, 13, 14, and 15 in the _totalRewardsClaimable array, it will encounter an index out of bounds error or potentially overwrite existing data in the array.
This line calculates the number of cycles between nextClaimableCvx and actualCycle (inclusive) and sets the length of the _totalRewardsClaimable array accordingly.
Going back to the example, if nextClaimableCvx is 10 and actualCycle is 15, then maxLengthRewards will be calculated as 15 - 10 + 1 = 6, which is the correct length to store the CVX rewards for all 6 cycles.
PlamenTSV
commented
1 month ago
Github username: -- Twitter username: -- Submission hash (on-chain): 0x66a4594cd04f9c4e80d1eb581fbfa2efc18032b360ecaf99bda6d1b2c40d10aa Severity: medium
Description: Description\ The line
uint256 maxLengthRewards = nextClaimableCvx;stores the accumulated CVX rewards from StakeDao for each ERC20 token.https://github.com/hats-finance/Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb/blob/246e3ac71f3f2e4ab7eded0f347ad8d070410262/contracts/Staking/Convex/StakingServiceBase.sol#L346
However, it is incorrect and leads to an incorrect calculation of the length of the _totalRewardsClaimable array.
The _totalRewardsClaimable array is used to store the CVX rewards for each ERC20 token across multiple cycles. The size of this array should be equal to the number of cycles between the next claimable cycle (nextClaimableCvx) and the current cycle (actualCycle), inclusive.
However, the current line uint256
maxLengthRewards = nextClaimableCvx;sets the length of the _totalRewardsClaimable array to be equal to nextClaimableCvx. This is incorrect because the function needs to calculate and store CVX rewards for multiple cycles, not just the nextClaimableCvx cycle.For example: Let's say nextClaimableCvx is 10, and actualCycle is 15. This means that the function needs to calculate and store CVX rewards for 6 cycles (10, 11, 12, 13, 14, and 15).
With the current line uint256 maxLengthRewards = nextClaimableCvx;, maxLengthRewards will be set to 10. This means that the _totalRewardsClaimable array will have a length of 10, which is not enough to store the CVX rewards for all 6 cycles.
As a result, when the function tries to store the CVX rewards for cycles 11, 12, 13, 14, and 15 in the _totalRewardsClaimable array, it will encounter an index out of bounds error or potentially overwrite existing data in the array.
The correct line should be:
This line calculates the number of cycles between nextClaimableCvx and actualCycle (inclusive) and sets the length of the _totalRewardsClaimable array accordingly.
Going back to the example, if nextClaimableCvx is 10 and actualCycle is 15, then maxLengthRewards will be calculated as 15 - 10 + 1 = 6, which is the correct length to store the CVX rewards for all 6 cycles.