Honest users could be permanently DOS'd from withdrawing their vested tokens/rewards

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/ChefIncentivesController.sol#L518

Vulnerability details

Proof of Concept

ChefIncentivesController#Claim() is a public function and callable by anyone.

When claiming there is a need to vest the tokens, now this vesting directly vests to the multifeedistributor, in the

In MultiFeeDistribution#vestTokens(), there is a logic to only allow pushing a new entry for a user to his _userEarnings array once in a day.

These rewards are then kept, and then after the vesting period a user can claim their rewards, where the _userEarnings array would be looped through.

Now from the links to the necessary functions attached, we can see that there is no minimum claimants to be made, asides the check that RDNT must be > 0, however as little as 1 is allowed.

So for active users that could earn even if little rewards per day, this then allows a malicious user to cause a permanent DOS to claiming their vested tokens, since all the attacker has to do, is once a day for as long as the vesting duration query, which could in valid scenarios be quite lengthy query ChefIncentivesController#Claim() while passing the user's address and then when the user comes to withdraw their vested tokens, the loop OOG's and then reverts due to the amount of entries in the _userEarnings array .

Impact

Potential permanent DOS to honest users [(or force the users to incur losses since some tech savvy users can notice this griefing attempt, but to stop this they'd have to withdraw early which forces them to incur losses)(https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/MultiFeeDistribution.sol#L505-L525)], since even if the vesting duration reduces there is no other method for the user to withdraw their tokens and it's stuck in the protocol, alternatively if the user leaves the claiming of their tokens even after the vesting period and does not immediately withdraw, the chances of this happening heavily increases since with each day an entry to the _userEarnings array can be made.

Recommended Mitigation Steps

Consider introducing some access control to ChefIncentivesController#Claim() and only allow the users call this for themselves.

Assessed type

DoS

[amarcu]

This is a grieve attack on rewards, the user funds are not at risk.

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report

[0xAlix2]

@koolexcrypto - I respectfully believe that this is invalid, as vesting tokens is only doable by minters, https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/MultiFeeDistribution.sol#L497C14-L497C21, that are assigned by the contract owner, i.e. trusted. Moreover, a user is allowed to call claim on a certain number of reward tokens, so even if DOS exists (it doesn't but assuming minters are malicious) a user can still claim their rewards.

[Bauchibred]

I do not understand the sponsors claim above to downgrade the severity of this report, afaik in the scope of this contest rewards are to be considered as users funds. This attack case has a simple path, a malicious user griefs users from their rewards by constantly calling ChefIncentivesController#Claim() so they are forced to incur losses, this directly puts their assets at risk as they can't access it as expected and why I submitted as High. Since the viable option for the users, which imo still points this to high is for them to withdraw early and incur losses.

Also I think @0xAlix2 seems to see the bug case as been backed by a malicious minter, however his claim actually shows how vesting the tokens every day would be what a non-malicious minter would do, since if users are active and earn rewards for that day then there should be no reason why their tokens are not vested for the day which is what he's suggesting, if that's done then this just breaks the logic as users are not being given their rewards, no?

[koolexcrypto]

Thank you everyone for your input. given the input above, I believe this is valid and stays as is