Lucene search

K
code423n4Code4renaCODE423N4:2023-07-POOLTOGETHER-FINDINGS-ISSUES-436
HistoryJul 14, 2023 - 12:00 a.m.

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

2023-07-1400:00:00
Code4rena
github.com
8
prizepool
accounting error
fee deduction
vulnerable code
manual review

Lines of code
<https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L473&gt;
<https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L830-L833&gt;
<https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L743-L746&gt;
<https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L312&gt;

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&gt;

    _transfer(_prizeRecipient, amount);

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

  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&gt;

  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&gt;

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

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

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


The text was updated successfully, but these errors were encountered:

All reactions