HonorLt - Not all providers claim the rewards

[sherlock-admin]

HonorLt

medium

Not all providers claim the rewards

Summary

Providers wrongly assume that the protocols will no longer incentivize users with extra rewards.

Vulnerability Detail

Among the current providers only the CompoundProvider claims the COMP incentives, others leave the claim function empty:

    function claim(address _aToken, address _claimer) public override returns (bool) {}

While many of the protocols currently do not offer extra incentives, it is not safe to assume it will not resume in the future, e.g. when bulls return to the town. For example, Aave supports multi rewards claim: https://docs.aave.com/developers/whats-new/multiple-rewards-and-claim When it was deployed on Optimism, it offered extra OP rewards for a limited time. There is no guarantee, but a similar thing might happen in the future with new chains and technology.

While Beta currently does not have active rewards distribution but based on the schedule, it is likely to resume in the future: https://betafinance.gitbook.io/betafinance/beta-tokenomics#beta-token-distribution-schedule

Impact

The implementations of the providers are based on the current situation. They are not flexible enough to support the rewards in case the incentives are back.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/AaveProvider.sol#L115

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/BetaProvider.sol#L122

Tool used

Manual Review

Recommendation

Adjust the providers to be ready to claim the rewards if necessary.

[mister0y]

While true, we can easily replace the providers when new functionality for claiming tokens becomes available.

[pauliax]

Escalate for 10 USDC.

While new providers can be added, old providers cannot be updated to accept these rewards, so users who were using old providers will get nothing and will be forced to migrate. Besides, deposits are not instantaneous, it takes time to rebalance them. Also, not all rewards are liquidity mining, some are snapshot-based. Let's say an external entity decides to airdrop all the Aave lenders. Currently, there is no way to claim it and distribute it among real users. This will incur lost rewards for the end users, thus I believe it is a valid concern.

[sherlock-admin]

Escalate for 10 USDC.

While new providers can be added, old providers cannot be updated to accept these rewards, so users who were using old providers will get nothing and will be forced to migrate. Besides, deposits are not instantaneous, it takes time to rebalance them. Also, not all rewards are liquidity mining, some are snapshot-based. Let's say an external entity decides to airdrop all the Aave lenders. Currently, there is no way to claim it and distribute it among real users. This will incur lost rewards for the end users, thus I believe it is a valid concern.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted

Given that there are no implementations for claims in some of the providers considering these a valid high.

[sherlock-admin]

Escalation accepted

Given that there are no implementations for claims in some of the providers considering these a valid high.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

[Theezr]

Fix: https://github.com/derbyfinance/derby-yield-optimiser/pull/201

[dmitriia]

Fix: derbyfinance/derby-yield-optimiser#201

The fix supplies providers with sendTokensToVault() functions, that retrieve arbitrary tokens from their balance, but currently these functions are supposed to be run manually by DAO, and not being systematically called from anywhere. claim() functions described in the issue are still empty. The absence of a better approach needs to be evaluated separately.