code-423n4 / 2022-11-redactedcartel-findings

3 stars 2 forks source link

Lack Of Proper Access Control Might Lead To User Getting Lesser Rewards #362

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexRewards.sol#L281-L295

Vulnerability details

Impact

We can call the function userAccrue for some other user and make their rewards lesser then they expect. In the function https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexRewards.sol#L281-L295 it calculates the rewards for a user that are being accrued over a period of time. The math to calculate how much reward a user has rewards accrued is uint256 rewards = u.rewards + u.lastBalance * (block.timestamp - u.lastUpdate); We can see from this equation that more the value of block.timestamp - u.lastUpdate more will be the rewards accrued.

Proof of Concept

1.) Bob decides to stake his producer token for 2 weeks (which would make his lastUpdate value after two weeks as two weeks) to get higher rewards 2.) Alice comes along , and after one week calls the function userAccrue with the same producer token and address of user bob in the user parameter. 3.) Now the lastUpdate for the user Bob after 2 weeks is not two weeks but one week because Alice called userAccrue on behalf of Bob after one week, and now Bob will get reward for one week instead of 2.

Tools Used

Manual analysis

Recommended Mitigation Steps

Most simple mitigation would be to add a check something like require(user == msg.sender) this way the user has to be the user himself who calls the function.

Picodes commented 1 year ago

Not an issue as rewards are saved with u.rewards = rewards;

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid