code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L188-L199

Vulnerability details

Impact

The setRewards function allows rewards to be set for specific epochs without checking if rewards have been established for all preceding epochs.

https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L188-L199

function setRewards(
    uint256 _fromEpoch,
    uint256 _toEpoch,
    uint248 _amountPerEpoch
) external is_valid_epoch(_fromEpoch) is_valid_epoch(_toEpoch) onlyGovernance {
    for (uint256 i = _fromEpoch; i <= _toEpoch; i += WEEK) {
        RewardInformation storage ri = rewardInformation[i];
        require(!ri.set, "Rewards already set");
        ri.set = true;
        ri.amount = _amountPerEpoch;
    }
}

The claim function permits rewards to be claimed starting primarily from the epoch when the user last claimed rewards.

https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L163

    uint256 claimStart = Math.max(userLastClaimed, _claimFromTimestamp);

This implies that if the rewards are set out of order, a user might be able to claim rewards for the latest weeks but will be unable to claim for prior weeks.

Proof of Concept

Note: For simplicity in this PoC, we're using conventional numbering for epochs. In the actual code, each epoch is represented by the previous epoch timestamp plus a week's worth of seconds.

Steps:

Governance Sets Rewards for Epochs 5-10: Call the setRewards function, defining the range as epochs 5 to 10 (in our conventional numbering).
User Claims Rewards for Epochs 5-10: The user interacts with the system and claims their rewards for epochs 5 to 10. The system will register the last epoch from which the user claimed rewards. In this case, the user's userLastClaimed variable will be updated to epoch 10 (again, using our conventional numbering).
Governance Sets Rewards for Epochs 1-4: Now, the Governance, for some reason, decides to set rewards for epochs 1 to 4 and does so by calling the setRewards function.
User Attempts to Claim Rewards for Epochs 1-4: Even though rewards for epochs 1 to 4 are now available, the user won't be able to claim them. This is due to the claim function's logic using Math.max(userLastClaimed, _claimFromTimestamp) to determine the starting point for reward claims. Given that userLastClaimed is epoch 10 (in our conventional numbering), the user will not be able to claim rewards from earlier epochs, even if they were set later on.

Tools Used

VS Code

Recommended Mitigation Steps

Implement an additional data structure (e.g., a mapping or an array) in the contract to keep track of the epochs for which a user has already claimed rewards. Each time a user claims rewards for an epoch, mark that epoch as "claimed" for that specific user. Modify the claim function to iterate over the epochs, checking the user's claim history. If an epoch is found for which the user hasn't claimed the rewards and rewards are available, allow the user to claim those rewards, irrespective of the userLastClaimed value. This will ensure that a user can claim rewards for any available epochs they haven't claimed yet.

Assessed type

Other

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #44

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)