Skip to content

Latest commit

 

History

History
126 lines (94 loc) · 7.61 KB

File metadata and controls

126 lines (94 loc) · 7.61 KB

Fun Bamboo Fly

Medium

Lack of Expiration Timestamp Check in GammaRewarder.sol#createDistribution()

Summary

The createDistribution() function in GammaRewarder.sol lacks an expiration timestamp check to verify that transactions have not remained in the mempool for an extended period. This exposes users to potential risk when submitting createDistribution() transactions, as the values of several core state variables (protocolFee, blocksPerEpoch, even isWhitelistedRewardToken) could change before the transaction is mined. These variables directly affect the rewards distribution and fees, potentially resulting in conditions users did not initially agree upon, such as a higher protocolFee.

[GammaRewarder.sol#createDistribution()](https://github.com/sherlock-audit/2024-10-gamma-rewarder/blob/main/GammaRewarder/contracts/GammaRewarder.sol#L102C1-L147C6) function:

/// @notice Incentivizor creates a new reward distribution
/// @param _hypervisor Address of the hypervisor to incentivize
/// @param _rewardToken Address of the reward token
/// @param _amount Total amount of tokens to distribute
/// @param _startBlockNum Starting block number for distribution
/// @param _endBlockNum Ending block number for distribution
function createDistribution(
    address _hypervisor, 
    address _rewardToken, 
    uint256 _amount, 
    uint64 _startBlockNum, 
    uint64 _endBlockNum
) external nonReentrant {
    require(_hypervisor != address(0), "Zero address not allowed");
    require(_rewardToken != address(0), "Zero address not allowed");
    require(_endBlockNum > _startBlockNum, "Invalid block range");
    require(_startBlockNum > block.number && (_endBlockNum - _startBlockNum) <= MAX_DISTRIBUTION_BLOCKS, "Distribution start block number is less than current block number or the duration is greater than 4 weeks.");
    require(_amount > 0, "Distribution Amount has to be greater than zero.");
    require((_endBlockNum - _startBlockNum) % blocksPerEpoch == 0, "Distribution length must be multiples of blocks per epoch");
    require(isWhitelistedRewardToken[_rewardToken] == 1, "Reward token has to be whitelisted.");
    require(IERC20(_rewardToken).allowance(msg.sender, address(this)) >= _amount, "Incentivisor did not approve enough tokens to Reward contract.");
    require(protocolFeeRecipient != address(0), "Zero address not allowed");

    uint256 fee = _amount * protocolFee / BASE_9;
    uint256 realAmountToDistribute = _amount - fee;
    uint256 amountPerEpoch = realAmountToDistribute / ((_endBlockNum - _startBlockNum) / blocksPerEpoch);

    IERC20(_rewardToken).safeTransferFrom(msg.sender, protocolFeeRecipient, fee);
    IERC20(_rewardToken).safeTransferFrom(msg.sender, address(this), realAmountToDistribute);

    uint256 senderNonce = nonces[msg.sender];
    bytes32 distributionId = bytes32(keccak256(abi.encodePacked(msg.sender, senderNonce)));
    DistributionParameters memory newDistribution = DistributionParameters({
        distributionId: distributionId,
        hypervisor: _hypervisor,
        rewardToken: _rewardToken,
        distributionAmountPerEpoch: amountPerEpoch,
        startBlockNumber: _startBlockNum,
        endBlockNumber: _endBlockNum,
        incentivizor: msg.sender
    });
    distributionList.push(newDistribution);
    distributions[distributionId] = newDistribution;
    nonces[msg.sender] = senderNonce + 1;
    emit NewDistribution(newDistribution, msg.sender);
}

Root Cause

The lack of a leak check in the createDistribution() function fails to ensure that critical state variables maintain the values ​​expected by users at the time the transaction is sent. Because the function relies on mutable state variables that can be modified before the mining of the transaction, there is a risk of executing with updated values ​​that users may find unacceptable. This opens the possibility for transactions to execute with outdated or unwanted parameters if they remain in the mempool for too long.

Internal pre-conditions

  1. Modifiable State Variables: The contract allows state variables that impact rewards distribution, such as protocolFee, blocksPerEpoch, and isWhitelistedRewardToken, to be modified between transaction submission and execution.
  2. Absence of Expiration Check: The function does not have a parameter (like a deadline) to ensure timely mining of the transaction or revert if the transaction lingers in the mempool too long.

External pre-conditions

  1. Transaction Delay in Mempool: The user submits a createDistribution() transaction that remains pending in the mempool for a prolonged period.
  2. State Variable Update by Contract Owner: During this time, the contract owner or authorized account modifies core state variables, such as increasing protocolFee, altering blocksPerEpoch, or updating the isWhitelistedRewardToken mapping.

Attack Path

Example with protocolFee state variable:

  1. The user submits a createDistribution() transaction based on the current contract state variables, including an acceptable protocolFee.
  2. The transaction lingers in the mempool for a very long time, allowing the contract owner time to change protocolFee or other parameters.
  3. When mined, the transaction executes with modified state variables, potentially applying an unintended or unacceptable fee rate or configuration.

Example with blocksPerEpoch state variable:

  1. A user submits a createDistribution() transaction intending a single large payout of 1,000,000 tokens across one epoch. At the time, blocksPerEpoch is set to 8,640,000 blocks, covering the full 4-week period.
  2. Due to network congestion or low gas fees, the transaction remains in the mempool for a significant amount of time.
  3. During this delay, blocksPerEpoch is updated to 43,200 blocks, splitting the 4-week duration into 200 smaller epochs.
  4. When the transaction is mined, the contract now distributes 5,000 tokens per epoch instead of one large payout, spreading rewards gradually instead of as a lump sum.
  • Now, the user receives smaller, incremental rewards rather than the expected one-time payout and distribution rate doesn’t match the user’s planned reward structure.

Impact

Without an expiration check, users risk having their transactions executed with changed state variables that may lead to unexpected or increased costs or unintended reward distribution conditions. For instance:

  1. Increased Protocol Fee: Users could be subject to a higher protocol fee than originally intended, impacting their distribution budget.
  2. Changed Epoch Timing: A modification to blocksPerEpoch could affect the distribution cadence, altering the per-epoch distribution amount and timing.
  3. Token Whitelisting: If a reward token's whitelisting status changes, users may unknowingly set up a distribution with an unwhitelisted token at the time of function execution (the particular example with Token Whitelisting is more low severity).

PoC

No response

Mitigation

Add a deadline parameter to the createDistribution() function, allowing users to specify the last acceptable block for the transaction to execute. If the transaction is mined after this block, it should revert:

function createDistribution(
    address _hypervisor, 
    address _rewardToken, 
    uint256 _amount, 
    uint64 _startBlockNum, 
    uint64 _endBlockNum,
    uint256 deadline // New deadline parameter
) external nonReentrant {
    require(block.number <= deadline, "Transaction expired");
    ...
}