_totalAssets() in Vault.sol calls the wrong function to get amount of assets managed by YieldVault

[code423n4]

Lines of code

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

Vulnerability details

Impact

The accounting of assets in Vault.sol will not be accurate. Exchange Rate Calculation will be affected.

Proof of Concept

In Vault.sol, maxWithdraw(address(this)) is used to calculate the amount of assets managed by the YieldVault.

   * @dev The total amount of assets managed by this vault is equal to
   *      the amount of assets managed by the YieldVault + the amount living in this vault.
  function _totalAssets() internal view returns (uint256) {
    return _yieldVault.maxWithdraw(address(this)) + super.totalAssets();
  }

maxWithdraw() in ERC4626 actually takes in the amount of shares that the owner has and calls _convertToAssets(). Calling maxWithdraw() and passing in address(this), the vault address, does not really work because the vault address do not hold the shares, the user does.

    function maxWithdraw(address owner) public view virtual returns (uint256) {
        return _convertToAssets(balanceOf(owner), Math.Rounding.Floor);
    }

    function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256) {
        return shares.mulDiv(totalAssets() + 1, totalSupply() + 10 ** _decimalsOffset(), rounding);
    }

Since the function _totalAssets() intends to return the asset amount in the yield vault. it should call totalAssets() to fetch the asset amount in the yieldVault instead of calling maxWithdraw().

    function totalAssets() public view virtual returns (uint256) {
        return _asset.balanceOf(address(this));
    }

This affects the exchangeRate calculation because _currentExchangeRate() also uses maxWithdraw(), but it should use totalAssets() instead.

    uint256 _withdrawableAssets = _yieldVault.maxWithdraw(address(this));

Tools Used

Manual Review

Recommended Mitigation Steps

Use totalAssets() instead to calculate the amount of assets in the Yield Vault.

Assessed type

ERC4626

[Picodes]

The goal is not fetch the total asset but how much poolTogether's vault could withdraw against its shares of the underlying vault

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid