The Standard

Summary

There is a current accepted collateral token, PAX Gold (PAXG), that charges a 0.02% fee on the amount of PAXG transferred. On LiquidationPool distributeAssets(), will calculate which portion of liquidated assets each staker gets. On the case of PAXG, since there is a cost of 0,02% on transfers, the amount that is attributed to stakers will be bigger than the amount that the LiquidationPool will receive from LiquidationPoolManager. This will mean, that there will not be enough liquidity to cover the calculated PAXG rewards, the first users will be able to receive their liquidated assets (LiquidationPool.claimRewards) but the last users will not be able to receive any rewards, and the function will revert.
Grading this finding as high, since PAXG, a fee on transfer token, is a current accepted collateral token, and this problem will occur if the upgrade goes through, as is.

Vulnerability Details

LiquidationManager.runLiquidation() will liquidate a vault and receive the liquidated assets. Then it will give approvals of the ERC20 assets to LiquidationPool, and call LiquidationPool.distributeAssets() with a list of the assets, and amounts currently on the LiquidationManager to be distributed to stakers.
LiquidationPool.distributeAssets() will calculate the portions for each stacker, based on the amounts received as parameter, and transfer the assets from the LiquidationPoolManager to the LiquidationPool. The amounts, for each asset, given to each staker will be recorded on the rewards mapping. Stakers can then pull the assets by calling LiquidationPool.claimRewards().

The problem is that the amount of PAXG received by parameter, and then split by the stakers will be bigger than the amount of PAXG that the LiquidationPool will receive, since there will be a fee on the transfer done at the end of distributeAssets function.

This will mean that the claimRewards will revert for the last stakers trying to pull their rewards, there will not be enough PAXG liquidity to complete the transfer.

Impact

Stakers awarded PAXG from liquidated assets will, possibly, not be able to claim any rewards, particularly if they are the last stakers to claim rewards. A complex solution, for a experienced user, could be managed, by transferring the amount PAXG needed and right after calling the LiquidationPool.distributeAssets(), but this behavior is definitely not expected.

Tools Used

Manual Review

Recommendations

Verify contract balance before and after transfer and see if it matches the expected transfer amount. If it does not match, equal the staker portion to be the amount that the contract received.

        if (asset.token.addr == address(0)) {
            nativePurchased += _portion;
        } else {
            assetBalanceBefore =  IERC20(asset.token.addr).balanceOf(address(this));
            IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
            assetBalanceAfter =  IERC20(asset.token.addr).balanceOf(address(this));
            transferedAmount = assetBalanceAfter - assetBalanceBefore;
            if (_portion != transferedAmount) {
                _portion = transferedAmount;
            } 
        }
        rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;