Summary & Vulnerability Details
distributeAssets() calculates _portion and then the costInEuros, which is the amount in EUROs the liquidator has to pay in exchange for the rewards or the collateral he helped liquidate. There are 2 issues with the following line of code which allows a liquidator to get 1 wei of collateral by paying 0 EUROs:
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
The complete function is as below:
LiquidationPool.sol#L219-L221
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
(,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
@-------------------> uint256 _portion = asset.amount * _positionStake / stakeTotal;
@-------------------> uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}
Issue-1:
Division before multiplication causes precision loss and hence liquidator can get away with paying nothing & still receive reward. Here's an example:
Let's consider ARB token which is officially supported by the protocol as an acceptable collateral. It has 18 decimal places.
It's Chainlink price feed puts the price at around (1 ARB = 1.9165 USD)
Let's suppose _portion to be equal to .
Chainlink price for EUR/USD pair has been taken in the tests as (1 EUR = 1.06 USD), so let's stick with that.
Then, costInEuros will be calculated by the protocol on L220 as:
$$
\begin{equation}
\begin{split} costInEuros &= _portion * {10} ^ {(18 - asset.token.dec)} * uint256(assetPriceUsd) / uint256(priceEurUsd) * _hundredPC / _collateralRate \
&= 1 * {10} ^ {(18 - 18)} * 191650000 / 106000000 * 100000 / 120000 \
&= 191650000 / 106000000 * 100000 / 120000 \
&= 1 * 100000 / 120000 \
&= 100000 / 120000 \
&= 0
\end{split}
\end{equation}
$$
One would imagine that doing all the multiplications before divisions would be a good fix. Let's check that:
$$
\begin{equation}
\begin{split} costInEuros &= _portion * {10} ^ {(18 - asset.token.dec)} * uint256(assetPriceUsd) * _hundredPC / (uint256(priceEurUsd) * _collateralRate) \
&= 1 * {10} ^ {(18 - 18)} * 191650000 * 100000 / (106000000 * 120000) \
&= 191650000 * 100000 / (106000000 * 120000) \
&= 19165000000000 / 12720000000000 \
&= 1
\end{split}
\end{equation}
$$
However, this is still incomplete and brings us to the issue:
Issue-2:
Imagine a price fluctuation such that 1 ARB < 1 EUR. This could easily happen in the above scenario with EUR/USD moving up, or ARB/USD moving down. Or, you can suppose an altogether different token which has price less than that of EUR. Then, due to costInEuros being rounded down instead of being rounded up, liquidator will get the reward for free.
Let's say 1 ARB = 1.059 USD or . Then -
$$
\begin{equation}
\begin{split} costInEuros &= _portion * {10} ^ {(18 - asset.token.dec)} * uint256(assetPriceUsd) * _hundredPC / (uint256(priceEurUsd) * _collateralRate) \
&= 1 * {10} ^ {(18 - 18)} * 105900000 * 100000 / (106000000 * 120000) \
&= 105900000 * 100000 / (106000000 * 120000) \
&= 10590000000000 / 12720000000000 \
&= 0
\end{split}
\end{equation}
$$
Hence the above should have been rounded up by using an external library like solmate which has the function mulDivUp or use custom logic as mentioned in the recommendation section below.
NOTE that the above vulnerability can be used to get free rewards of more than by mounting an attack vector which uses multiple accounts causing _portion to calculate as 1. Consider the following scenario showcasing an attack scenario which will drain all ARB collateral while paying nothing:
Malicious liquidator stakes TST & EUROs from 100 different accounts, each account staking .
ARB collateral to be liquidated
For each account (holder), _portion calculated as which is credited to the account holder.
Combined rewards from all accounts = , with EUROs paid.
PoC
Use the following patch to add the test to test/liquidationPoolManager.js and run via npx hardhat test --grep 'free liquidation gain' to see it pass.
In this test since there is only 1 holder, and ARB collateral to be liquated is 1, _portion will calculate to 1 just as in the above examples and result in "free liquidation":
@@ -160,6 +160,37 @@ describe('LiquidationPoolManager', async () => {
await expect(LiquidationPoolManager.runLiquidation(TOKEN_ID)).to.be.revertedWith('vault-not-undercollateralised');
});
+ it('free liquidation gain', async () => {
+ ARB = await ERC20MockFactory.deploy('Arbitrum', 'ARB', 18)
+ const ArbUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('ARB/USD');
+ await ArbUsd.setPrice(191650000);
+ await TokenManager.addAcceptedToken(ARB.address, ArbUsd.address);
+
+ const arbCollateral = 1;
+
+ // create some funds to be "liquidated"
+ await ARB.mint(SmartVaultManager.address, arbCollateral);
+
+ const tstStake1 = ethers.utils.parseEther('1000');
+ const initialEURObalance = ethers.utils.parseEther('2000');
+ const eurosStake1 = initialEURObalance;
+ await TST.mint(holder1.address, tstStake1);
+ await EUROs.mint(holder1.address, eurosStake1);
+ await TST.connect(holder1).approve(LiquidationPool.address, tstStake1);
+ await EUROs.connect(holder1).approve(LiquidationPool.address, eurosStake1);
+ await LiquidationPool.connect(holder1).increasePosition(tstStake1, eurosStake1);
+ let { _rewards, _position } = await LiquidationPool.position(holder1.address);
+ expect(_position.EUROs).to.equal(eurosStake1);
+
+ await fastForward(DAY);
+ await LiquidationPoolManager.runLiquidation(TOKEN_ID);
+ expect(await ARB.balanceOf(LiquidationPool.address)).to.equal(arbCollateral);
+
+ ({ _rewards, _position } = await LiquidationPool.position(holder1.address));
+ expect(_position.EUROs).to.equal(initialEURObalance); // @audit-info : no EUROs paid by liquidator
+ expect(rewardAmountForAsset(_rewards, 'ARB')).to.equal(arbCollateral); // @audit-info : received the collateral
+ });
+
it('distributes liquidated assets among stake holders if there is enough EUROs to purchase', async () => {
const ethCollateral = ethers.utils.parseEther('0.5');
const wbtcCollateral = BigNumber.from(1_000_000);
Impact
Tools Used
Hardhat
Recommendations
Round up costInEuros by using an external library like solmate which has the function mulDivUp or use custom logic as mentioned below:
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
(,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
uint256 _portion = asset.amount * _positionStake / stakeTotal;
- uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
- * _hundredPC / _collateralRate;
+ uint256 costInEuros = (_portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) * _hundredPC) / (uint256(priceEurUsd) * _collateralRate); // @audit : multiply before dividing
+ if (costInEuros * uint256(priceEurUsd) * _collateralRate < _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) * _hundredPC) costInEuros++; // @audit : round-up
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}
