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

12 stars 7 forks source link

Possibility of incorrect balance calculation in `_accountedBalance()` Function #137

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Inaccurate Accounting: The incorrect balance calculation can disrupt the accurate accounting of tokens within the protocol. This can make it challenging to track and monitor token balances correctly, potentially leading to financial discrepancies and misreporting of token allocations.

Proof of Concept

The _accountedBalance() function calculates the balance of tokens that have been accounted for. However, the vulnerability lies in the subtraction operation (obs.available + obs.disbursed) - _totalWithdrawn. If the subtraction operation results in an underflow, it can have adverse consequences.

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

Tools Used

manual

Recommended Mitigation Steps

To mitigate this vulnerability, it is crucial to perform a check to ensure that the subtraction operation does not result in a negative value. One way to address this is by adding a require statement to verify that (obs.available + obs.disbursed) is greater than or equal to _totalWithdrawn before the subtraction. If the condition is not met, the transaction should be reverted to prevent further execution.

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

  require(totalBalance >= _totalWithdrawn, "Balance calculation error: Underflow");

  return totalBalance - _totalWithdrawn;
}

Assessed type

Under/Overflow

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid