Unsafe conversion may lead to Theft of unclaimed yield
Summary
There is an unsafe conversion between uint256 to uint96RewardsManager.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 uint96lastInteractionBurnEpoch will be equal to 0
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
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 claimRewardslastInteractionBurnEpoch will be set to 0 thus re-calculating the reward from the start each time.
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
Blockian
medium
Unsafe conversion may lead to Theft of unclaimed yield
Summary
There is an unsafe conversion between
uint256
touint96
RewardsManager.sol
that may cause Theft of unclaimed yieldVulnerability Detail
On
_claimRewards
inRewardsManager
contract thelastInteractionBurnEpoch
is being converted fromuint256
touint96
,lastInteractionBurnEpoch
is being used in the reward calculation to decide from what epoch should the reward calculation start, thus by passing for example the value79228162514264337593543950336
which is1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
in binary, after the conversion touint96
lastInteractionBurnEpoch
will be equal to 0The next code can be found in
_calculateAndClaimRewards
andcalculateRewards
Impact
Currently, due to the fact that
epochToClaim_
is not limited bycurrentBurnEpoch
to cause this bug will required lots of gas. BUT considering a naive fix to theepochToClaim_
issue may result in allowing malicious users to receive the same reward multiple times as each call toclaimRewards
lastInteractionBurnEpoch
will be set to0
thus re-calculating the reward from the start each time.Recommendation
If
epochToClaim_
is always converted to uint96 anyway just receive it from the users as auint96