c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L350 https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L364 https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L382

Vulnerability details

Impact

PrizeVault doesn't strictly follow the rules for an EIP4626 contract. The following functions are non-compliant:

convertToAssets
maxDeposit
maxMint

Proof of Concept

EIP4626 states the following:

convertToAssets MUST NOT revert unless due to integer overflow caused by an unreasonably large input.

However we can see that in PrizeVault::convertToAssets will revert if totalDebt_ is 0 (due to division by zero error). This can happen for example if all the users withdrew their balances and claimed rewards and yieldFeeBalance > _asset.balanceOf(address(this)).

maxDeposit MUST NOT rely on balanceOf of asset

And we can see the maxDeposit function in PrizeVault:

function maxDeposit(address) public view returns (uint256) {
    uint256 _totalSupply = totalSupply();
    uint256 totalDebt_ = _totalDebt(_totalSupply);
    if (totalAssets() < totalDebt_) return 0;

    // the vault will never mint more than 1 share per asset, so no need to convert supply limit to assets @audit is it true??
    uint256 twabSupplyLimit_ = _twabSupplyLimit(_totalSupply);
    uint256 _maxDeposit;
    uint256 _latentBalance = _asset.balanceOf(address(this));
    uint256 _maxYieldVaultDeposit = yieldVault.maxDeposit(address(this));
    if (_latentBalance >= _maxYieldVaultDeposit) {
        return 0;
    } else {
        unchecked {
            _maxDeposit = _maxYieldVaultDeposit - _latentBalance;
        }
        return twabSupplyLimit_ < _maxDeposit ? twabSupplyLimit_ : _maxDeposit;
    }
}

And indeed it depends on _latentBalance which is the balanceOf.