The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Error in Accumulating Holder's Fees on Staked EUROs, Causing Misrepresentation of Funds

dyoff

[M-01] Error in Accumulating Holder's Fees on Staked EUROs, Causing Misrepresentation of Funds

Description

The LiquidationPool::position() function provides a snapshot of staked EUROs and TST along with accumulated fees and rewards. However, a flaw exists in the calculation process within this function. Specifically, at line 88, the accrued fees in EUROs for holders based on their staked TST are miscalculated. The issue arises from considering the entire balance of the LiquidationPoolManager rather than deducting the LiquidationPoolManager::poolFeePercentage and allocating it to the protocol.

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(); // @audit not deducting the fee
_rewards = findRewards(_holder);
}

The correct calculation, outlined in LiquidationPoolManager::distributeFees() at line 35, involves deducting the LiquidationPoolManager::poolFeePercentage from the manager's total balance before distributing fees to the holders by calling LiquidationPool::distributeFees() and then forwaring the remaining to the protocol

function distributeFees() public {
IERC20 eurosToken = IERC20(EUROs);
@> uint256 _feesForPool = eurosToken.balanceOf(address(this)) * poolFeePercentage / HUNDRED_PC; // @audit deducting the fee
if (_feesForPool > 0) {
eurosToken.approve(pool, _feesForPool);
LiquidationPool(pool).distributeFees(_feesForPool);
}
eurosToken.transfer(protocol, eurosToken.balanceOf(address(this)));
}

The erroneous formula currently used is:

Impact

The miscalculation in displaying holder's assets can lead to an unfavorable off-chain user experience (UX).

Proof of Concept

Proof of Code written with Foundry can be found here. This test shows the difference of funds of a holder when calling LiquidationPool::position() before and after distribution of the fees.

Recommended Mitigation

Adjust the LiquidationPool::position() function to incorporate the LiquidationPoolManager::poolFeePercentage variable obtained from the manager, which will forward the needed funds to the holders and the others to the protocol's multisig. Here's an edited version with the suggested changes:

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) _position.EUROs += (IERC20(EUROs).balanceOf(manager) * poolFeePercentage) * _position.TST / getTstTotal();
_rewards = findRewards(_holder);
}

Tools Used

Manual review

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

incorrect-position

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.