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

Malicious user can easily DoS Liquidation Pool by increasing unique holders via calling `increasePosition` with dust amount

chainnue

Summary

Malicious user can easily DoS Liquidation Pool by increasing unique holders by increasePosition with dust amount resulting distributeFees, distributeAssets dos-ed.

Vulnerability Details

There are several occasions this for (uint256 i = 0; i < holders.length; i++) is being used in core function, like distributeFees, distributeAssets. As gas factor is an important factor in evm, there is a limitation on looping of the holders.

Attacker can flooded this holders easily by sending a dust amount of TST or EUROs via calling increasePosition from many unique addresses. This will resulting a drastic increases of unique holders var.

Attacker can very easily generate 1000 address, and distribute 1 WEI of TST or EUROs into those addresses, and programatically call increasePosition (assuming there were gases to exec trx on those addresses)

File: LiquidationPool.sol
112: function addUniqueHolder(address _holder) private {
113: for (uint256 i = 0; i < holders.length; i++) {
114: if (holders[i] == _holder) return;
115: }
116: holders.push(_holder);
117: }
118:
...
134: function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
135: require(_tstVal > 0 || _eurosVal > 0);
136: consolidatePendingStakes();
137: ILiquidationPoolManager(manager).distributeFees();
138: if (_tstVal > 0) IERC20(TST).safeTransferFrom(msg.sender, address(this), _tstVal);
139: if (_eurosVal > 0) IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _eurosVal);
140: pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));
141: addUniqueHolder(msg.sender);
142: }
...
182: function distributeFees(uint256 _amount) external onlyManager {
183: uint256 tstTotal = getTstTotal();
184: if (tstTotal > 0) {
185: IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
186: for (uint256 i = 0; i < holders.length; i++) {
187: address _holder = holders[i];
188: positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
189: }
190: for (uint256 i = 0; i < pendingStakes.length; i++) {
191: pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;
192: }
193: }
194: }

If we look at distributeFees it will loop into holders length, which will be out of gas when the size is big.

Impact

DoS of LiquidationPool, affecting distributeFees, distributeAssets availability

Tools Used

Manual Analysis

Recommendations

Consider to add a constraint like minimum position size, or redesign the logic to replace the for loop.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year 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.