145.md

Early Red Tuna

Medium

Wrong error description in 'require' will confuse users when trying to claim

Summary

Wrong error description in 'require' will confuse users when trying to claim

Root Cause

In line 203 of GammaRewarder.sol, the claiming period (determined by decoded startBlock and endBlock from _appCircuitOutput) must be included in the distribution period (recovered from DistributionParameters struct associated to decoded distributionId). This is the code line of handleProofResult() function with issue:

require(startBlock >= params.startBlockNumber && endBlock <= params.endBlockNumber, "Claim range has to include distribution range.");

https://github.com/sherlock-audit/2024-10-gamma-rewarder/blob/main/GammaRewarder/contracts/GammaRewarder.sol#L187-L218

If this is fulfilled, user will be able to claim without further problem. However, if this is not fulfilled then user will receive an error message that will lead them to confusion, encouraging them to repeat the function call as the asked condition is apparently being met, making them think that the contract does not allow them to claim when they should be able to claim.

Internal pre-conditions

1. A distribution is created with startBlockNumber = 100 and endBlockNumber = 200.

External pre-conditions

No response

Attack Path

1. Alice calls brevisCallback() with parameters of the ZK-proof that allows them to be rewarded the distributed tokens. Decoded from _appCircuitOutput parameters are startBlock = 90 and endBlock = 210. The mentioned require line require(startBlock >= params.startBlockNumber && endBlock <= params.endBlockNumber, "Claim range has to include distribution range."); will make the call revert as 90 > 100 && 210 < 200 is clearly not fulfilled.
2. Alice receives the error message "Claim range has to include distribution range.", which confuses her as the distribution range is clearly included in the claim range decoded from circuit output.
3. She tries to claim again with same input parameters for brevisCallback() function , as she is sure that it fulfills the required conditions.
4. Alice receives the same error once and again and will end up believing the contract is incorrectly designed and does not operate as intended, giving up to claim her tokens.

Impact

Users will in some cases receive a wrong error message, that leads them to try to claim once and again as apparently the distribution range and claim range comply with what the contract requires. They will not know that for being able to claim, the claim range must be included in the distribution range.

PoC

No response

Mitigation

Correct the error message thrown when this require is not fulfilled, so that users clearly know why the function reverts:

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(startBlock >= params.startBlockNumber && endBlock <= params.endBlockNumber, "Claim range has to be included in the 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.");