search

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

3 stars 2 forks source link

DoS on claiming rewards in `PirexRewards` is possible #387

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/9e9bb60f117334da7c5d851646a168ca271575fc/src/PirexRewards.sol#L373

Vulnerability details

Proof of Concept

The claim method in PirexRewards iterates over the rewardTokens array for a producerToken. Now this array is completely managed by the contract’s owner who can call addRewardToken which pushes a new value in that array, as many times as he decides with whatever value he decides.

Let’s look at the following scenario:

  1. Owner has turned malicious or is compromised
  2. Owner calls addRewardToken a huge amount of times, which results in the rewardTokens array being huge
  3. Now when a user calls claim the code will take a crazy amount of gas, because it has to iterate over the whole rewardTokens array. Since the amount of gas will possibly be more than the block gas limit, this results in a DoS and the user won’t be able to claim.

Or this scenario:

  1. Owner has turned malicious or is compromised
  2. Owner calls addRewardToken with an address that is not really a token
  3. Now when a user calls claim the code will try to do a call to producer.claimUserReward giving the address of the rewardToken, but since it is not really a token (or maybe it is just a random address with no bytecode) the call will revert. Actually it will always revert, which is a DoS state.

Impact

This is a centralisation vulnerability allowing the owner to stop the user rewards anytime. Since it requires a malicious/compromised owner it is Medium severity

Recommendation

Add a way for the user to claim rewards only from a token he chooses, not to have to go through all reward tokens on each claim.

drahrealm commented 1 year ago

Related to issue #271 and also a centralization issue.

c4-sponsor commented 1 year ago

drahrealm marked the issue as disagree with severity

Picodes commented 1 year ago

Although I do agree with the warden's mitigation that it'd be interesting for users to have the option to claim only for the token he chooses, currently the call to claimUserReward does not revert if the token address is incorrect, so the call would not revert. Duplicate #271 with partial credit as the warden identified the weak point but the scenario described are very unlikely or do not work

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #271

c4-judge commented 1 year ago

Picodes marked the issue as partial-50