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

DoS 24 after a user spam `pendingStakes` with small amounts, until `consolidatePendingStakes` would cost more than block gas limit

nmirchev8

Summary

Inside LiquidationPool.sol users can stake their TST or EURO and earn rewards. The contract has a functionality to store pendingStakes for 24 hours, before officially include them to the user's position mapping. The problem comes from the complexity of the function consolidatePendingStakes, which is crucial for the protocol and the fact that user can call unlimited times increasePosition with 1 wei for amount.

Vulnerability Details

  • The problem comes from the complexity of consolidatePendingStakes, which iterates over all pending stakes twice if 24 hours has passed.

  • A user can in one transaction create 200 pending stakes with same timestamp for createdAt, which would make function consolidatePendingStakes massive gas cost exactly 24 hours after user “attack start”. For all 200 pending transactions deletePendingStake would be called, which would read & write > 100 times storage variables. This is close to 100^2 complexity, which for only SSTORE operation = 100^2 * **20,000 = 200 000 000 , when block gas limit for Mainnet is only 30 000 000, which is clearly DoS.**

PoC

In the following gist I have provided coded PoC and instructions how to execute it
https://gist.github.com/NicolaMirchev/0322a697c3170958585086f02ce08411

Impact

  • consolidatePendingStakes is used inside: increasePosition , decreasePosition & distributeAssets , which means DoS of those function. And the whole staking system is dependant to those functions, which is really bad. Noone would be able to withdraw his rewards & staked funds.

  • Gas cost for the attacker is relatively low

Tools Used

  • Manual Review

  • Hardhat

Recommendations

  • Limit pendingStakes per user:

+ mapping(address => bool) private pendingStakes;
​
function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
+ require(!pendingStake[msg.sender], "User has a pending stake");
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));
+ pendingStake[msg.sender] = true;
addUniqueHolder(msg.sender);
}
​
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));
+ pendingStake[msg.sender] = false;
// pause iterating on loop because there has been a deletion. "next" item has same index
i--;
}
}
}
Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

pendingstake-dos

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.