code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1068-L1073

Vulnerability details

Impact

Prize claims can be executed by external actors to get some fees as a reward. These fees are dynamic and are limited in range, and some hooks will be executed before and after the claim.

As the fees are deducted from the prizeTotal received by the winner, they could be incentivized to create an afterClaimPrize hook that reverts the transaction when fees are too high, to maximize the total profit.

As the transaction is executed by the claimer, the winner doesn't lose anything by doing so, and at most, they can wait until the last moment before their prize expires to disable the hook.

This results in a very bad experience for claimers as their transaction will revert and they will waste gas, possibly eroding trust in the claiming system.

Proof of Concept

claimPrize is executed between two hooks:

if (hooks.useBeforeClaimPrize) {
  recipient = hooks.implementation.beforeClaimPrize(_winner, _tier, _prizeIndex);
} else {
  recipient = _winner;
}

uint prizeTotal = _prizePool.claimPrize(
  _winner,
  _tier,
  _prizeIndex,
  recipient,
  _fee,
  _feeRecipient
);

if (hooks.useAfterClaimPrize) {
  hooks.implementation.afterClaimPrize(
    _winner,
    _tier,
    _prizeIndex,
    prizeTotal - _fee,
    recipient
  );
}

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1052-L1075

A vault can check the fee paid for this claim by calculating the delta between prizeTotal and prizeTotal - _fee, as they are part of the same transaction.

If this delta is too high, the vault might revert the transaction inside the afterClaimPrize hook until the market conditions are better.

Tools Used

Manual review

Recommended Mitigation Steps

Consider removing the afterClaimPrize hook to avoid this scenario.

Assessed type

DoS

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #465