The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Eliminate unnecessary casting and simplify the `LiquidationPool#consolidatePendingStakes()` code.

0xaadhi

Vulnerability Details

In this consolidatePendingStakes function, the variable i is initialized as int256 to handle potential negative values during the decrement (See line number 129). Additionally, the code requires casting int256(i) to uint256(i) to use the index i.

​
File: contracts/LiquidationPool.sol
​
119: function consolidatePendingStakes() private {
120: uint256 deadline = block.timestamp - 1 days;
121: for (int256 i = 0; uint256(i) < pendingStakes.length; i++) {
122: PendingStake memory _stake = pendingStakes[uint256(i)];
123: if (_stake.createdAt < deadline) {
124: positions[_stake.holder].holder = _stake.holder;
125: positions[_stake.holder].TST += _stake.TST;
126: positions[_stake.holder].EUROs += _stake.EUROs;
127: deletePendingStake(uint256(i));
128: // pause iterating on loop because there has been a deletion. "next" item has same index
129: @> i--;
130: }
131: }
132: }

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L119C1-L132C6

Tools Used

Manual Review

Recommendations

This unnecessary casting can be removed by adding an else block, as shown in the code below.

function consolidatePendingStakes() private {
uint256 deadline = block.timestamp - 1 days;
- for (int256 i = 0; uint256(i) < pendingStakes.length; i++) {
+ for (uint256 i = 0; i < pendingStakes.length;) { // @audit no increment here
- PendingStake memory _stake = pendingStakes[uint256(i)];
+ PendingStake memory _stake = pendingStakes[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--;
+ deletePendingStake(i);
+ // @audit The loop variable 'i' is not incremented here because deletePendingStake shifts the indexes.
+ } else {
+ i++; // @audit Only increment 'i' if no deletion occurs.
}
}
}

The loop variable i is only incremented if no deletion occurs. If a PendingStake is deleted, the loop continues without incrementing i, ensuring that the next item is not skipped. This approach maintains the correct iteration over the pendingStakes array even as items are removed.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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