`_totalWithdrawn` VALUE DOES NOT INCLUDE THE `_fee` AMOUNT THUS INTRODUCING ACCOUNTING ERROR

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L459 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L473 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L830-L833 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L743-L746 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L312

Vulnerability details

Impact

In the PrizePool.ClaimPrize function is used to claim the rewards of the verified winner. Here when sending the Prize amount to the winner a _fee amount is deducted from it as shown below:

uint256 amount = tierLiquidity.prizeSize - _fee;

And this amount is transferred to the winner of the draw by calling the _transfer functin as shown below:

_transfer(_prizeRecipient, amount);

In the PrizePool._transfer function the _totalWithdrawn state variable is updated with the prize amount as shown below:

_totalWithdrawn += _amount;

But the issue here is that the _totalWithdrawn is updated without the _fee amount. Hence the _totalWithdrawn does not indicate the total liquidity that has been withdrawn (in the form of prizes) from the beginning.

The _totalWithdrawn is used in the PrizePool._accountedBalance function to calculate the number of tokens that have accounted for. But this calculation is wrong since the _fee amount is excluded in the _totalWithdrawn value.

Since the _accountedBalance is used inside the Vault.liquidate function for a conditional check in calculating the Prize Pool tokens to be transferred to the accumulator, the above error could propagate to break the accounting of the critical functions and their conditional checks.

Proof of Concept

    uint256 amount = tierLiquidity.prizeSize - _fee;

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L459

    _transfer(_prizeRecipient, amount);

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L473

  function _transfer(address _to, uint256 _amount) internal {
    _totalWithdrawn += _amount;
    prizeToken.safeTransfer(_to, _amount);
  }

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L830-L833

  function _accountedBalance() internal view returns (uint256) {
    Observation memory obs = DrawAccumulatorLib.newestObservation(totalAccumulator);
    return (obs.available + obs.disbursed) - _totalWithdrawn;
  }

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L743-L746

    uint256 _deltaBalance = prizeToken.balanceOf(address(this)) - _accountedBalance();

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L312

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to update the _totalWithdrawn inside the PrizePool.ClaimPrize function with the total Prize amount before the _fee is deducted as shown below:

_totalWithdrawn += tierLiquidity.prizeSize; 

Assessed type

Other

[asselstine]

The fee hasn't been withdrawn yet; the fee is withdrawn in this function: withdrawClaimRewards().

You can see that it calls the _transfer function, which increases the total withdrawn.

[c4-sponsor]

asselstine marked the issue as sponsor disputed

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid