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

The `decreasePosition` function could remove a holder even if there are pending stake

0x9527

Summary

When a user invokes decreasePosition, the function checks the current holder's position. If the position is empty, the current holder will be popped from the holders array. However, if there is a pending stake by the current holder, this pending staker will be added to the position with a 1-day duration. The next time the manager invokes distributeAssets and loops through the current holders array, it may not find this holder even if they have an active position.

Vulnerability Details

Position holder invoke decreasePosition to derease his positon:

function decreasePosition(uint256 _tstVal, uint256 _eurosVal) external {
consolidatePendingStakes();
ILiquidationPoolManager(manager).distributeFees();//@note distribute fees in manager contract.
​
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]);//@audit delete position not calculate the pending token.
}

and function check if current holder's position is empty

function empty(Position memory _position) private pure returns (bool) {
return _position.TST == 0 && _position.EUROs == 0;
}

Note that it's a storage variable of position only check the amount of position without check the pending staker.

Assuming Alice invokes increasePosition to add a position twice, and after one day, Alice invokes decreasePosition. Since Alice's position is now ZERO, her address could be popped from the holders array, even if Alice has a pending stake. The positions() function will return the current position along with the pending position. Alice's pending position will be added to the position after some time; however, we won't find Alice's address in the holders array if she doesn't increase her position again.

Here is my test written using foundry:

function testDecrasePosition() public {
uint256 tstAmount = 1000;
uint256 euroAmount = 2000;
​
deal(address(TST),alice,tstAmount);
deal(address(EUROs),alice,euroAmount);
vm.startPrank(alice);
IEUROs(EUROs).approve(address(lp),type(uint256).max);
IERC20(TST).approve(address(lp),type(uint256).max);
​
lp.increasePosition(11, 11);
vm.warp(block.timestamp + 1);
​
address FirstHolder = lp.holders(0);
assertEq(FirstHolder,alice);
console2.log("FirstHolder",FirstHolder);
​
lp.increasePosition(100, 100);
vm.warp(block.timestamp + 1 days);
lp.decreasePosition(11, 11);
​
//get alice's position info,
(,uint256 tst,uint256 euro) = lp.positions(alice);
console2.log("position TST",IERC20(TST).balanceOf(alice));
console2.log("position EUROs",IEUROs(EUROs).balanceOf(alice));
​
//get first holders
vm.expectRevert();
FirstHolder = lp.holders(0);
}

output:

Running 1 test for test/LiquidationPool.t.sol:LiquidationTest
[PASS] testDecrasePosition() (gas: 688228)
Logs:
FirstHolder 0x328809Bc894f92807417D2dAD6b7C998c1aFdac6
position TST 900
position EUROs 1900
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.20ms

As we can see the length of holders is ZERO even if alice has position. Lead to next distributeFees or distributeAssets can't find alice's position address.

Let's take distributeFees as example :

function distributeFees(uint256 _amount) external onlyManager {
uint256 tstTotal = getTstTotal();
if (tstTotal > 0) {
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
for (uint256 i = 0; i < holders.length; i++) {
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;//@audit lost precision.
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;//@note add EUROs fees based on TST amount in position and pending position.
}
}
}

When distributing fees, if a user is not present in the holders array, and they increase their position within one day, their stake tokens are added to the position. As a result, their stake tokens are not included in the pending stake. Consequently, when fees are distributed, the user receives no reward.

Impact

staker lost distribute fees and assets.

Tools Used

Foundry

Recommendations

recommend to check pending stake along with position stake before delete holder.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years 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.