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

12 stars 7 forks source link

Calculation Inaccuracy in `_totalAssets()` Function #134

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

If an overflow occurs during the addition, the result will wrap around to a smaller value, leading to an inaccurate total assets calculation. This can affect subsequent operations and potentially cause unexpected behavior in the contract.

Proof of Concept

The _totalAssets() function calculates the total assets managed by the contract's vault. Here's the vulnerable code snippet:

function _totalAssets() internal view returns (uint256) {
  return _yieldVault.maxWithdraw(address(this)) + super.totalAssets();
}

Tools Used

manual

Recommended Mitigation Steps

SafeMath libraries offer functions for performing arithmetic operations with built-in checks to prevent overflow and underflow.

import "@openzeppelin/contracts/utils/math/SafeMath.sol";

// ...

function _totalAssets() internal view returns (uint256) {
  uint256 maxWithdraw = _yieldVault.maxWithdraw(address(this));
  uint256 currentAssets = super.totalAssets();

  return SafeMath.add(maxWithdraw, currentAssets);
}

Assessed type

Under/Overflow

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 1 year ago

There is a really high number of low-quality reports by this warden