Summary
The increasePosition function inside the LiquidationPool contract can be used by stakers to increase their stake. The stake is not increased instantly, but instead the request to do so is pushed into a pendingStakes array which acts as a queue to increase the stake and waits there for at least 24 hours before the stake is increased while already acquiring fees.
The decreasePosition function can be used to decrease the stake. If the stake is decreased to 0, the position is deleted. It is not checked if there is a pending stake left before deleting the position. Which leads to lost fees and funds for the user.
Vulnerability Details
Here we can see the decreasePosition function which is called by the user to decrease the stake. The function first calls consolidatePendingStakes which removes the pending stake (if older than 24 hours) and adds the stake to the position. If the stake is decreased to 0, the position is deleted and the holder is deleted from the holders array. It is not checked if there is a pending stake left before deleting the position:
function decreasePosition(uint256 _tstVal, uint256 _eurosVal) external {
consolidatePendingStakes();
ILiquidationPoolManager(manager).distributeFees();
require(_tstVal <= positions[msg.sender].TST && _eurosVal <= positions[msg.sender].EUROs, "invalid-decr-amount");
if (_tstVal > 0) {
IERC20(TST).safeTransfer(msg.sender, _tstVal);
positions[msg.sender].TST -= _tstVal;
}
if (_eurosVal > 0) {
IERC20(EUROs).safeTransfer(msg.sender, _eurosVal);
positions[msg.sender].EUROs -= _eurosVal;
}
if (empty(positions[msg.sender])) deletePosition(positions[msg.sender]);
}
function empty(Position memory _position) private pure returns (bool) {
return _position.TST == 0 && _position.EUROs == 0;
}
function deletePosition(Position memory _position) private {
deleteHolder(_position.holder);
delete positions[_position.holder];
}
function deleteHolder(address _holder) private {
for (uint256 i = 0; i < holders.length; i++) {
if (holders[i] == _holder) {
holders[i] = holders[holders.length - 1];
holders.pop();
}
}
}
As we can see in the distributionFees function, the fees are distributed to all holders in the holders array. If the user is not in the holders array, the fees are lost:
function distributeFees(uint256 _amount) external onlyManager {
uint256 tstTotal = getTstTotal();
if (tstTotal > 0) {
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
for (uint256 i = 0; i < holders.length; i++) {
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;
}
}
}
Therefore, calling decreasePosition again for the rest of the pending stake after decreasing the stake to 0 will first call consolidatePendingStakes which removes the pending stake and adds the stake to the position, but the holder is still not in the holders array and after that distributeFees is called which adds fees to all holders in the holders array. As the holder is not in the holders array, the fees are lost for the user.
As the distributeFees function calculates with the total amount of TST the fees are stuck in the contract till the next distributeAssets call which will forward them to the wallet of the protocol.
The following POC can be implemented in the liqudationPool test file and showcases how the fees acquired in the pending stake are lost:
describe("decrease position can steal fees from the user", async () => {
it("decrease position can steal fees from the user", async () => {
const tstStake = ethers.utils.parseEther("2000");
const tstPortion = ethers.utils.parseEther("1000");
await TST.mint(user1.address, tstStake);
await TST.mint(user2.address, tstStake);
await TST.connect(user1).approve(LiquidationPool.address, tstStake);
await TST.connect(user2).approve(LiquidationPool.address, tstStake);
await LiquidationPool.connect(user1).increasePosition(tstPortion, 0);
await LiquidationPool.connect(user2).increasePosition(tstPortion, 0);
await fastForward(DAY);
await LiquidationPool.connect(user1).increasePosition(tstPortion, 0);
await LiquidationPool.connect(user2).increasePosition(tstPortion, 0);
await LiquidationPool.connect(user1).decreasePosition(tstPortion, 0);
const fees = ethers.utils.parseEther("20");
await EUROs.mint(LiquidationPoolManager.address, fees);
await fastForward(DAY);
await LiquidationPool.connect(user2).decreasePosition(
tstStake,
ethers.utils.parseEther("10")
);
await expect(
LiquidationPool.connect(user1).decreasePosition(
tstPortion,
ethers.utils.parseEther("10")
)
).to.be.revertedWith("invalid-decr-amount");
await TST.connect(user1).approve(LiquidationPool.address, tstPortion);
await LiquidationPool.connect(user1).increasePosition(tstPortion, 0);
await fastForward(DAY);
await expect(
LiquidationPool.connect(user1).decreasePosition(
tstPortion,
ethers.utils.parseEther("10")
)
).to.be.revertedWith("invalid-decr-amount");
LiquidationPool.connect(user1).decreasePosition(tstStake, 0);
expect(await TST.balanceOf(user1.address)).to.equal(
await TST.balanceOf(user2.address)
);
expect(await EUROs.balanceOf(user1.address)).to.equal(0);
expect(await EUROs.balanceOf(user2.address)).to.be.gt(0);
});
});
The same issue occurs in the distributeAssets function:
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);
}
Impact
Fees and assets which belong to the user are accidentally stolen.
Recommendations
Only delete the position if there are no pending stakes left.
