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

User can have an active `position` in `LiquidationPool` even without being in the `holders` array`

0xmakeouthill

Summary

A user can have a position in LiquidationPool without being in the holders array. Which will lead to discrepancy in getStakeTotal and getTstTotal, since they iterate over the holders array.

Vulnerability Details

function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
​
require(_tstVal > 0 || _eurosVal > 0);
​
consolidatePendingStakes();
​
ILiquidationPoolManager(manager).distributeFees();
​
if (_tstVal > 0) IERC20(TST).safeTransferFrom(msg.sender, address(this), _tstVal);
​
if (_eurosVal > 0) IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _eurosVal);
​
pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));
​
addUniqueHolder(msg.sender);
​
}

  1. User calls increasePosition.
    Which pushes his stake as pending:

pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal)); and adds him as an unique holder

function addUniqueHolder(address _holder) private {
​
for (uint256 i = 0; i < holders.length; i++) {
​
if (holders[i] == _holder) return;
​
}
​
holders.push(_holder);
​
}

Now since there are other pending stakes and also a user must wait 1 day before his pending stake passes the deadline period in order to be added in a position. He can call decreasePosition immediately after that:

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

consolidatePendingStakes(); won't add his pending stake in position because it hasn't passed the deadline and also there might be other pending stakes.

In order to pass this require require(_tstVal <= positions[msg.sender].TST && _eurosVal <= positions[msg.sender].EUROs, "invalid-decr-amount"); just set the input amounts to 0.

Then at the end we get to 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;
}

Which will return true because our pending stake hasn't been added yet to our position and we will go into the if statement, which will delete our position.

function deletePosition(Position memory _position) private {
​
deleteHolder(_position.holder);
delete positions[_position.holder];
}

This deletes as a holder and also deletes our position but since we don't have anything added there yet it's not a problem.

Time passes and other users invoke consolidatePendingStakes and finally we get our pending stake added to our position without being in the holders array.

function consolidatePendingStakes() private {
uint256 deadline = block.timestamp - 1 days;
​
for (int256 i = 0; uint256(i) < pendingStakes.length; i++) {
​
PendingStake memory _stake = pendingStakes[uint256(i)];
​
if (_stake.createdAt < deadline) {
​
positions[_stake.holder].holder = _stake.holder;
​
positions[_stake.holder].TST += _stake.TST;
​
positions[_stake.holder].EUROs += _stake.EUROs;
​
deletePendingStake(uint256(i));
​
// pause iterating on loop because there has been a deletion. "next" item has same index
i--;
​
}
​
}
​
}

Impact

Discrepancy will be made in getStakeTotal and getTstTotal since they assume that only holders can have active positions.

Which in return won't accumulate the correct fees and rewards for stakers, because they are all dependent on these two functions.

Tools Used

Manual Audit

Recommendations

When consolidatePendingStakes is called it may be a good idea to check if a user has been removed as an holder and add him back if that's the case.

Updates

Lead Judging Commences

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

deletePosition-issye

Support

FAQs

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