joshuajee - The `handleProofResult` function doesn't keep track of the amount that has been claimed in a distribution, this will lead to a particular distribution claiming more funds than what was deposited. #210
The handleProofResult function doesn't keep track of the amount that has been claimed in a distribution, this will lead to a particular distribution claiming more funds than what was deposited.
Summary
Anyone can create distribution in the contract, and all the funds to be distributed are stored in the contract, so at any time there can be 10 different distributions running in parallel each with its own balance and the total stored in the contract. The problem with this is that the handleProofResult function that is supposed to claim rewards from a particular distribution doesn't check or reduce the distribution balance before sending the rewardToken, because of this it is possible for funds greater than the distribution balance to be taken from the contract.
Root Cause
The root cause is in the handleProofResult function, there is nowhere that the totalRewardAmount is compared to the distributionAmountPerEpoch, also the total amount that has been distributed per epoch is not recorded, and more funds can be distributed in a particular epoch, worst a distribution can distribute more funds than the one that was deposited when creating such distribution.
function handleProofResult(bytes32, bytes32 _vkHash, bytes calldata _appCircuitOutput) internal override {
require(vkHashes[_vkHash], "invalid vk");
(
address userAddress,
address lpTokenAddress,
uint64 startBlock,
uint64 endBlock,
bytes32 distributionId,
address rewardTokenAddress,
uint248 distributionAmountPerEpoch,
uint248 totalRewardAmount
) = decodeOutput(_appCircuitOutput);
DistributionParameters memory params = distributions[distributionId];
require(startBlock < endBlock && (endBlock - startBlock) % blocksPerEpoch == 0, "Claim period must be valid");
require(startBlock >= params.startBlockNumber && endBlock <= params.endBlockNumber, "Claim range has to include distribution range.");
require(lpTokenAddress == params.hypervisor && rewardTokenAddress == params.rewardToken && distributionAmountPerEpoch == params.distributionAmountPerEpoch, "Distribution params must match");
require(totalRewardAmount > 0, "Rewards do not exist for you.");
// Closing reentrancy gate here
CumulativeClaim memory claim = claimed[userAddress][rewardTokenAddress][distributionId];
require(claim.amount == 0 , "Already claimed reward.");
claim.startBlock = startBlock;
claim.endBlock = endBlock;
claim.amount = totalRewardAmount;
claimed[userAddress][rewardTokenAddress][distributionId] = claim;
IERC20(rewardTokenAddress).safeTransfer(userAddress, totalRewardAmount);
emit Claimed(userAddress, distributionId, rewardTokenAddress, startBlock, endBlock, totalRewardAmount);
}
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
Distribution will take spend more than what was intended for it to spend, by eating into the funds reserved for other distributions.
Other distributions will not be able to fully share rewards because some other distributions have eaten into their deposit.
PoC
No response
Mitigation
Track the amount that has been distributed inside the handleProofResult function, and check if the tracked amount is less than the amount that should be distributed.
joshuajee
Medium
The
handleProofResult
function doesn't keep track of the amount that has been claimed in a distribution, this will lead to a particular distribution claiming more funds than what was deposited.Summary
Anyone can create distribution in the contract, and all the funds to be distributed are stored in the contract, so at any time there can be 10 different distributions running in parallel each with its own balance and the total stored in the contract. The problem with this is that the
handleProofResult
function that is supposed to claim rewards from a particular distribution doesn't check or reduce the distribution balance before sending therewardToken
, because of this it is possible for funds greater than the distribution balance to be taken from the contract.Root Cause
The root cause is in the
handleProofResult
function, there is nowhere that thetotalRewardAmount
is compared to thedistributionAmountPerEpoch
, also the total amount that has been distributed per epoch is not recorded, and more funds can be distributed in a particular epoch, worst a distribution can distribute more funds than the one that was deposited when creating such distribution.https://github.com/sherlock-audit/2024-10-gamma-rewarder/blob/main/GammaRewarder/contracts/GammaRewarder.sol#L204
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
PoC
No response
Mitigation
Track the amount that has been distributed inside the
handleProofResult
function, and check if the tracked amount is less than the amount that should be distributed.