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

Incorrect value returns by "position" function

0x392

Summary

"position" function returns inaccurate values after "deletePendingStake" has been called at least once.

Vulnerability Details

In "LiquidationPool.sol" the function "deletePendingStake":

function deletePendingStake(uint256 _i) private {
for (uint256 i = _i; i < pendingStakes.length - 1; i++) {
pendingStakes[i] = pendingStakes[i+1];
}
pendingStakes.pop();
}

shifts elements in the "pendingStakes" array when deleting a stake which causes the order of stakes in the array to change.

Lets look at the "position" function:

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);
}

It retrieves information about the pending stakes of a holder by calling the function "holderPendingStakes".

"holderPendingStakes":

function holderPendingStakes(address _holder) private view returns (uint256 _pendingTST, uint256 _pendingEUROs) {
for (uint256 i = 0; i < pendingStakes.length; i++) {
PendingStake memory _pendingStake = pendingStakes[i];
if (_pendingStake.holder == _holder) {
_pendingTST += _pendingStake.TST;
_pendingEUROs += _pendingStake.EUROs;
}
}
}

iterates over the "pendingStakes" array, assuming a certain order of stakes. If the order in the array has changed due to deletions, the function might not calculate correctly "_pendingTST" and "_pendingEUROs" for a given holder. This means that when updating "_position.EUROs" and "_position.TST", the function "position" will use inaccurate pending stakes data, leading to incorrect results in the final "_position" struct.

Impact

Users won't be able to see an accurate representation of their positions

Tools Used

manual review

Recommendations

Consider modifying "holderPendingStakes" to check each element in the "pendingStakes" array without assuming a specific order. One approach could be using a mapping to store pending stakes for each holder, allowing for efficient retrieval without relying on array order.
Or consider a different method for deleting pending stakes that doesn't change the order of the elements in the array. One way would be by swapping the stake to be deleted with the last element in the array and then popping the last element. Here is an example of how it could look:

function deletePendingStake(uint256 _i) private {
if (_i < pendingStakes.length - 1) {
pendingStakes[_i] = pendingStakes[pendingStakes.length - 1];
}
pendingStakes.pop();
}
Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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