Summary
The LiquidationPool::Position(...) function incorrectly calculates the position of a staker, potentially leading to conflicts.
Vulnerability Details
The issue lies in the flawed calculation of the staker's position within the LiquidationPool::Position(...) function. Specifically, the calculation includes the fee rewards of the user, but the fee reward calculation itself is flawed. The function considers the euro token balance of the liquidation pool manager and calculates rewards based on the staker's TST balance in the position (both consolidated and pending). However, this approach results in an inaccurate representation of the staker's position.
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);
}
GitHub: [83-90]
The problem arises from the fact that the staker's proportion is calculated based on the entire amount stored in the manager, while only a percentage of this amount is transferred from the manager's balance to the pool balance as a fee reward.
function distributeFees() public {
IERC20 eurosToken = IERC20(EUROs);
uint256 _feesForPool = eurosToken.balanceOf(address(this)) * poolFeePercentage / HUNDRED_PC;
if (_feesForPool > 0) {
eurosToken.approve(pool, _feesForPool);
LiquidationPool(pool).distributeFees(_feesForPool);
}
eurosToken.transfer(protocol, eurosToken.balanceOf(address(this)));
}
GitHub: [33-41]
This discrepancy results in the stake balance shown by the LiquidationPool::Position(...) function being higher than the actual position, potentially causing issues for any function or external protocol that relies on this information.
Impact
Can cause integration problem and loss of tokens if this function is used in critical functions.
Proof of Concept
NOTE
To setup and run the test, please read the details from readme file in the github: repo
Here is a test that proves that:
The test verifies that the LiquidationPool::Position(...) function returns an incorrect position of a staker.
function test_PositionFunctionReturnsIncorrectPositionOfStaker() public {
uint256 amount = 1000 ether;
tokens.tstToken.mint(alice, amount * 2);
tokens.eurosToken.mint(alice, amount * 2);
vm.startPrank(alice);
tokens.tstToken.approve(address(contracts.liquidationPool), amount);
tokens.eurosToken.approve(address(contracts.liquidationPool), amount);
contracts.liquidationPool.increasePosition(amount, amount);
vm.stopPrank();
(address vault, uint256 tokenIdMinted) = _createSmartVault(bob);
assertEq(contracts.smartVaultManagerV5.ownerOf(tokenIdMinted), bob, "Bob is not the owner");
tokens.arbToken.mint(bob, amount);
vm.startPrank(bob);
tokens.arbToken.transfer(vault, amount);
vm.stopPrank();
uint256 bobArbBalanceInVault = tokens.arbToken.balanceOf(vault);
assertEq(bobArbBalanceInVault, amount, "Vault Balance is not correct");
vm.startPrank(bob);
uint256 eurosBobWant = 100 ether;
SmartVaultV3(payable(vault)).mint(bob, eurosBobWant);
vm.stopPrank();
uint256 bobEurosBalance = tokens.eurosToken.balanceOf(bob);
assertEq(bobEurosBalance, eurosBobWant, "Bob's Euro Balance is not correct");
uint256 feeGenerated = (eurosBobWant * constants.PROTOCOL_FEE_RATE) / 100000;
uint256 expectedFeeGenerated = 0.5 ether;
uint256 eurosFeeInLiquidationManager = tokens.eurosToken.balanceOf(address(contracts.liquidationPoolManager));
assertEq(eurosFeeInLiquidationManager, feeGenerated, "fee is not equal");
assertEq(feeGenerated, expectedFeeGenerated, "fee generated is not same");
skip(block.timestamp + 3 days);
(LiquidationPool.Position memory alicePosition, ) = contracts.liquidationPool.position(alice);
uint256 feeThatWillBeReceivedByAliceAccordingToPositionFunction = alicePosition.EUROs - amount;
vm.startPrank(alice);
tokens.tstToken.approve(address(contracts.liquidationPool), amount);
tokens.eurosToken.approve(address(contracts.liquidationPool), amount);
contracts.liquidationPool.increasePosition(amount, amount);
vm.stopPrank();
(alicePosition, ) = contracts.liquidationPool.position(alice);
uint256 feeActuallyReceivedByAlice = alicePosition.EUROs - (amount * 2);
vm.expectRevert();
require(feeActuallyReceivedByAlice == feeThatWillBeReceivedByAliceAccordingToPositionFunction, "fee is not equal");
assertEq(feeActuallyReceivedByAlice *2, feeThatWillBeReceivedByAliceAccordingToPositionFunction, "fee is not equal");
}
Link to original Test: Link
Output:
Running 1 test for test/unit/Unit.t.sol:Unit
[PASS] test_PositionFunctionReturnsIncorrectPositionOfStaker() (gas: 3739347)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 13.51ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Tools Used
Recommendations
It is recommended to make the following changes to the LiquidationPool::Position(...) function. Note that corresponding changes should be made to the ILiquidationPoolManager interface.
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();
+ if (_position.TST > 0){
+ uint256 rewardsEuro = (IERC20(EUROs).balanceOf(manager) * ILiquidationPoolManager(manager).poolFeePercentage()) / ILiquidationPoolManager(manager).HUNDRED_PC();
+ _position.EUROs += (rewardsEuro * _position.TST) / getTstTotal();
+ }
_rewards = findRewards(_holder);
}
