code423n4 commented 1 year ago

Lines of code

Vulnerability details

Original Issue

M-14 - Number of prize tiers always increases if just 1 canary prize is claimed

Details

The original issue was caused by an incorrect logic when computing the next number of tiers, the problem was that the next number of tiers was always updated even though it was not required to expand the number of tiers.

The issue was because the _nextNumberOfTiers variable was updated before determining if it was required to expand the number of tiers, and if it was not required, the function returned the updated value of the _nextNumberOfTiers, thus, causing the number of tiers to expand even when it was not required.

Mitigation

The mitigation simplified the tier expansion algorithm, and now, depending on the number of prizeCount claimed for the current draw it is determined the number of tiers for the next draw, the number of tiers is capped by the MAXIMUM_NUMBER_OF_TIERS (10).

function _computeNextNumberOfTiers(uint32 _claimCount) internal view returns (uint8) {
  // claimCount is expected to be the estimated number of claims for the current prize tier.
  uint8 numTiers = _estimateNumberOfTiersUsingPrizeCountPerDraw(_claimCount);
  return numTiers > MAXIMUM_NUMBER_OF_TIERS ? MAXIMUM_NUMBER_OF_TIERS : numTiers; // add new canary tier
}

function _estimateNumberOfTiersUsingPrizeCountPerDraw(uint32 _prizeCount) internal view returns (uint8) {
  if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_4_TIERS) {
    return 3;
  } else if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_5_TIERS) {
    return 4;
  } else if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_6_TIERS) {
    return 5;
  } else if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_7_TIERS) {
    return 6;
  } else if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_8_TIERS) {
    return 7;
  } else if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_9_TIERS) {
    return 8;
  } else if (_prizeCount < ESTIMATED_PRIZES_PER_DRAW_FOR_10_TIERS) {
    return 9;
  }
  return 10;
  }

Conclusion

The implemented mitigation solves the original issue.

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory