PoolTogether - kutugu's results

A protocol for no-loss prize savings

General Information

Platform: Code4rena

Start Date: 07/07/2023

Pot Size: $121,650 USDC

Total HM: 36

Participants: 111

Period: 7 days

Judge: Picodes

Total Solo HM: 13

Id: 258

League: ETH

PoolTogether

Findings Distribution

Researcher Performance

Rank: 59/111

Findings: 1

Award: $140.30

QA:
grade-a

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

140.2996 USDC - $140.30

Labels

bug
downgraded by judge
grade-a
QA (Quality Assurance)
sponsor confirmed
Q-02

External Links

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/DrawAccumulatorLib.sol#L454-L456

Vulnerability details

Impact

The binarySearch does not check for overlap between the left and right index of the sliding window, and loops indefinitely when satisfies item not exists in list until revert or gas is exhausted.

Proof of Concept

    while (true) {
      // We start our search in the middle of the `leftSide` and `rightSide`.
      // After each iteration, we narrow down the search to the left or the right side while still starting our search in the middle.
      currentIndex = (leftSide + rightSide) / 2;

      beforeOrAtIndex = uint16(RingBufferLib.wrap(currentIndex, _cardinality));
      beforeOrAtDrawId = _drawRingBuffer[beforeOrAtIndex];

      afterOrAtIndex = uint16(RingBufferLib.nextIndex(currentIndex, _cardinality));
      afterOrAtDrawId = _drawRingBuffer[afterOrAtIndex];

      bool targetAtOrAfter = beforeOrAtDrawId <= _targetLastClosedDrawId;

      // Check if we've found the corresponding Observation.
      if (targetAtOrAfter && _targetLastClosedDrawId <= afterOrAtDrawId) {
        break;
      }

      // If `beforeOrAtTimestamp` is greater than `_target`, then we keep searching lower. To the left of the current index.
      if (!targetAtOrAfter) {
        rightSide = currentIndex - 1;
      } else {
        // Otherwise, we keep searching higher. To the left of the current index.
        leftSide = currentIndex + 1;
      }
    }

According to the code only targetAtOrAfter &&_targetLastClosedDrawId <= afterOrAtDrawId can break loop, if no element in the list meets the condition, it loops indefinitely until the array index overflows or exhausts gas

Tools Used

Manual review

Should check leftSide > rightSide to break the loop

Assessed type

Loop

#0 - c4-sponsor

2023-07-20T23:11:16Z

asselstine marked the issue as sponsor confirmed

#1 - Picodes

2023-08-07T16:27:23Z

There is a small chance of wasted gas due to this search never ending, but the report doesn't dig into the potential impact of this finding. Downgrading to Low.

#2 - c4-judge

2023-08-07T16:27:27Z

Picodes changed the severity to QA (Quality Assurance)

#3 - c4-judge

2023-08-08T14:31:10Z

Picodes marked the issue as grade-a

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter