The Standard

Summary

LiquidationPool::distributeAssets can be exploited so that one staker steals all the rewards. This happens because the contract doesn't keep track of the msg.value send.

Vulnerability Details

LiquidationPool::distributeAssets can be called with msg.value = 0 and still update the rewards mapping. In the next section I will demonstrate a scenerio where an attacker can take advantage of this vulnerability and gain control of all the rewards.

Exploit scenario

For simplicity let's say that we have 2 stakers. Staker 1 has 10 000TST and 5 000EUROs. Staker 2 has the same amount of tokens.
Now if one of the stakers is malicious he can exploit distributeAssets function:

1. Staker 1 (he is the malicious one) calls LiquidationPool::distributeAssets and he provides the following arguments:

    let assets = [{
      token: {
        symbol: , // The symbol of the native token
        addr: ethers.constants.AddressZero,
        dec: 18, // Token decimals
        clAddr: , // Chainlink price feed address
        clDec: 8 // Chainlink result decimals
      },
      amount: 1e18
    }]
    await LiquidationPool.distributeAssets(assets, 110000, 100000);
   

That means we are calling the function providing 0 eth but in the assets struct we set the amount of the native token to 1e18. Now since staker 1 and staker 2 have the same staked amount we get the portion of each staker to 0.5 ether. We update rewards mapping.

                    rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;

Since we are distributing native token the address of that token is 0 so we update nativePurchased

                    if (asset.token.addr == address(0)) {
                        nativePurchased += _portion;
                    }

At the end of the function nativePurchased = 1e18 and asset.amount = 1e18 so we don't send the manager any eth since _assets[i].amount - _nativePurchased = 0

2. Now both stakers have a total reward of 1 eth (0.5 eth each) but no one can claim that reward because LiquidationPool balance is 0

3. After some time a liquidation has happened and someone executes runLiquidation and the value send to the LiquidationPool is 1e18 of native tokens

4. We update rewards mapping again and now each staker has 1 eth of value

5. Staker 1 immediately sees the transaction and back runs it by calling LiquidationPool::claimRewards and since he has 1 eth stored in the rewards mapping and the total balance of the contract is 1 eth, the transaction goes successfully and the ether balance is sent to staker 1. By doing that staker 1 successfully steals all the rewards of other stakers.

Impact

Stealing rewards

Tools Used

Manual Review

Recommendations

I have two suggestions:

1. You can either restrict LiquidationPool::distributeAssets to only be called by the LiquidityPoolManager by adding a modifier onlyManager.

2. You can keep track of the msg.value by making sure at the end of the function that msg.value = nativePurchased