The Standard

Summary

A staker won't be able to claim his rewards + a part of his stake if the call to him fails.

Vulnerability Details

LiquidationPool::claimRewards does a call to the user if there are any native tokens in rewards. However that call can fail. For example if the user did the deposit from a contract that does not accept ether (native tokens). Let's look at the following scenario to understand the issue better:

1. Bob deposits both TST and EURO tokens through a contract he created that would help him interact with the protocol, however he didn't implement a receive or fallback function which would make all the calls that send value revert.

2. A vault has been liquidated and native tokens and other accepted erc20 tokens are distributed to all the stakers including Bob. The issue here becomes bigger as Bob will not be able to claim his rewards but a part of his stake will also be unclaimable. This happens because of this line:

                            _position.EUROs -= costInEuros;
   

This decreases Bob's position with the euro value of the native tokens that he received divided by the collateralRate. Now the rewards mapping contains both his rewards + a part of his stake (costInEuros * hundredPC / collateralRate).
Here are some calculations:

1. Bob has 100 EURO collateral
2. Bob gets ditributed 10 EURO of rewards (this is his part)
3. rewards[token, Bob] = 10 EURO
4. costInEuros = 10 EURO * 100/110 = 9 EURO
5. Bob now has 91 EURO of collateral
6. Bob nets 1 EURO

But since Bob cannot claim his rewards that means his staked EURO becomes from 100 EURO tokens to 91 EURO tokens. He loses 9 EURO tokens. The loss becomes even bigger if a lot of other erc20 tokens are distributed alongside with the native tokens. Then his EURO staked amount can potentially become 0. That means Bob loses all of his staked EURO tokens.

3. Bob tries to claim his rewards by calling claimRewards through his custom smart contract. This call will fail because Bob has rewards in native tokens and since this function loops through all the allowed tokens, we would try to send Bob's contract a value that cannot be accepted. Bob will not only not be able to claim his native rewards but all of the erc20 tokens as well meaning his loss becomes quite significant.

Impact

This issue results in loss of funds and stuck tokens.

Tools Used

Manual Review

Recommendations

I would recommend to make it possible for users to claim their rewards individually for each token. This solves the problem with not being able to claim all of the ERC20 tokens, but the issue with not being able to claim the native tokens still exists. To solve this I recommend using the rewards mapping to only store the reward and not rewards + costInEuros which takes from user's staked EURO tokens. By doing that at least the user doesn't lose any of his staked tokens and loses the reward only but since he didn't implement a way to receive it, it is his fault.