Summary
The LiquidationPool::position() is used to return TST & sEURO staked amount for users. However, the calculation is wrong. When the user wants to decrease their position and withdraw their tokens, they are unable to do so, because the sEURO token amount returned called from "position" is doubled. So whenever user specifies the returned sEURO token amount returned from LiquidationPool::position() into the LiquidationPool::decreasePosition() it will always revert. If other contracts were to call from the LiquidationPool::position(), it will always display the wrong amount, which will impact the other contracts.
Vulnerability Details
The main problems come from the formula:
function getTstTotal() private view returns (uint256 _tst) {
for (uint256 i = 0; i < holders.length; i++) {
_tst += positions[holders[i]].TST;
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
_tst += pendingStakes[i].TST;
}
}
...
function position(address _holder) external view returns(Position memory _position, Reward[] memory _rewards) {
_position = positions[_holder];
(uint256 _pendingTST, uint256 _pendingEUROs) = holderPendingStakes(_holder);
_position.EUROs += _pendingEUROs;
_position.TST += _pendingTST;
if (_position.TST > 0) _position.EUROs += IERC20(EUROs).balanceOf(manager) * _position.TST / getTstTotal();
_rewards = findRewards(_holder);
}
PoC
Elaboration of user tries to withdraw the amount reflected from the LiquidationPool::position(). Furthermore, I chose high severity because of the visibility in LiquidationPool::getTstTotal(). Users are not able to calculate the total TST tokens, which means that they do not know how much tokens they actually have.
describe('Decrease Position Withdrawal PoC', async () => {
it('User not allowed to withdraw full amount of tokens due to position return wrong amount of tokens', async () => {
let tstStakeValue = ethers.utils.parseEther('10000');
await TST.mint(user1.address, tstStakeValue);
await EUROs.mint(user1.address, tstStakeValue);
await TST.connect(user1).approve(LiquidationPool.address, tstStakeValue);
await EUROs.connect(user1).approve(LiquidationPool.address, tstStakeValue);
await LiquidationPool.connect(user1).increasePosition(tstStakeValue, tstStakeValue);
const fees = ethers.utils.parseEther('100');
await EUROs.mint(LiquidationPoolManager.address, fees);
let { _position } = await LiquidationPool.position(user1.address);
console.log(_position)
console.log("here")
expect(_position.EUROs).to.equal(ethers.utils.parseEther('10100'));
await fastForward(DAY);
await expect(LiquidationPool.connect(user1).decreasePosition(tstStakeValue, ethers.utils.parseEther('10100'))).to.be.revertedWith('invalid-decr-amount');
await LiquidationPool.connect(user1).decreasePosition(tstStakeValue, ethers.utils.parseEther('10050'));
expect(await TST.balanceOf(user1.address)).to.equal(ethers.utils.parseEther('10000'));
expect(await EUROs.balanceOf(user1.address)).to.equal(ethers.utils.parseEther('10050'));
});
});
Impact
Users are not able to withdraw the full amount and decrease their position in the LiquidationPool.
Tools Used
Manual Review
Recommendations
Since 50% of the tokens will be transferred from LiquidatioNPoolManager and to the LiquidationPool and the other is sent to the multisig treasury, I recommend to include in the position formula the division of two.
uint256 halfAmount = IERC20(EUROs).balanceOf(manager) / 2;
if (_position.TST > 0) _position.EUROs += halfAmount * _position.TST / getTstTotal();
