code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

High Prizes might not be claimed #415

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L210 https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L173

Vulnerability details

Impact

PoolTogether V5 delegates the task of claiming the prizes to a network of Claimers that are incentivized to claim prizes for a share of the prize won. The fees the Claimant receives is calculated based on an Dutch Auction, but the limited by the minimum prize of the price pool

fee = min( fee in auction, % maxFeePortionOfPrize * minPrize)

When the gas costs exceed the minPrize the solver has not incentives to claim the price. Notice that the number of prizes only adjusts after very few prizes are claimed.

Proof of Concept

Gas prices rise well above the minPrize. The highest prize is suddenly drawn that day. The solvers have no incentive to claim the prize and the winner does not monitor the contract. As a result, no one claims the top prize. The protocol suffers a loss of reputation.

Tools Used

Manual review, VS Code

Recommended Mitigation Steps

The problem is that the incentives are not exactly aligned. The user is willing to pay anything up to his price p to receive his price. But the maximum that the solvers receives is the minimum price. We can align the two incentives by using each user's price as an upper bound.

function _computeMaxFee(uint8 _tier, uint8 _numTiers) internal view returns (uint256) {
    uint8 _canaryTier = _numTiers - 1;
    if (_tier != _canaryTier) {
      // canary tier
-     return _computeMaxFee(prizePool.getTierPrizeSize(_canaryTier - 1));
+     return _computeMaxFee(prizePool.getTierPrizeSize(_tier));
    } else {
      return _computeMaxFee(prizePool.getTierPrizeSize(_canaryTier));
    }
  }

Since we already validate the inputs in claimPrize the attacker can not claim more than the prize is worth.

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-sponsor commented 1 year ago

asselstine marked the issue as sponsor confirmed

c4-judge commented 11 months ago

Picodes marked the issue as satisfactory

c4-judge commented 11 months ago

Picodes marked the issue as selected for report

asselstine commented 11 months ago

Fixed here https://github.com/GenerationSoftware/pt-v5-claimer/pull/7