sherlock-audit / 2023-01-ajna-judging

1 stars 0 forks source link

Blockian - Unsafe conversion may lead to Theft of unclaimed yield #137

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Blockian

medium

Unsafe conversion may lead to Theft of unclaimed yield

Summary

There is an unsafe conversion between uint256 to uint96 RewardsManager.sol that may cause Theft of unclaimed yield

Vulnerability Detail

On _claimRewards in RewardsManager contract the lastInteractionBurnEpoch is being converted from uint256 to uint96, lastInteractionBurnEpoch is being used in the reward calculation to decide from what epoch should the reward calculation start, thus by passing for example the value 79228162514264337593543950336 which is 1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 in binary, after the conversion to uint96 lastInteractionBurnEpoch will be equal to 0

Impact

Currently, due to the fact that epochToClaim_ is not limited by currentBurnEpoch to cause this bug will required lots of gas. BUT considering a naive fix to the epochToClaim_ issue may result in allowing malicious users to receive the same reward multiple times as each call to claimRewards lastInteractionBurnEpoch will be set to 0 thus re-calculating the reward from the start each time.

Code Snippet

// ...
stakeInfo.lastInteractionBurnEpoch = uint96(epochToClaim_);
// ...

The next code can be found in _calculateAndClaimRewards and calculateRewards

        uint256 lastBurnEpoch = stakes[tokenId_].lastInteractionBurnEpoch; // will equal 0 always

        //...

        for (uint256 epoch = lastBurnEpoch; epoch < epochToClaim_; ) { // will iterate from 0 

Tool used

Manual Review

Recommendation

If epochToClaim_ is always converted to uint96 anyway just receive it from the users as a uint96

    function claimRewards(
        uint256 tokenId_,
        uint96 epochToClaim_
    ) external override