code-423n4 / 2023-03-neotokyo-findings

4 stars 0 forks source link

User can claim high rewards than he eligible #440

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1432-L1435 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1331 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1342 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1355-L1357 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1378

Vulnerability details

Impact

User will receive rewards more than he should receive calculation of rewards for user can be continued even after withdraw

Proof of Concept

when a user claim rewards, the lastRewardTime will be set to block.timestamp , now consider a scenario that user claims and withdraw all of his tokens and after a long time stake some tokens , since lastRewardTime have been set to block.timestamp of claim rewards time , then during claiming rewards of new staked tokens, sinceRewardTime will be calculated in a way like he has been staking new amount since lastRewardTime .

Scenario : 1 - User stake some tokens 2 - after endlocktime he claim rewards and withdraw all of his tokens . 3 - after a time stake new amount of tokens . 4 - when endlocktime of new staked tokens arrive he claim rewards . 5 - rewards has been calculated from the time of last claimed reward time due that in period between last claim reward and staking new tokens he hadn't any staked amount but that time is considered for time of staking .

Tools Used

Manual Review

Recommended Mitigation Steps

check that if user withdraw all funds set lastRewardTime to zero and a check in getPoolRewards function to ensure if lastRewardTime is zero not amount rewards apply for it .

hansfriese commented 1 year ago

getReward() will be called before he stakes again.

c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Invalid