PoolTogether - RedTiger's results

Lines of code

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

Vulnerability details

Impact

Anyone can steal the YieldFee available in Vault.sol

Proof of Concept

The function mintYieldFee is external and everybody can call it to mint shares. The only condition is that the number of _shares should be lower or eaqual than _yieldFeeTotalSupply

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

 function mintYieldFee(uint256 _shares, address _recipient) external {
    _requireVaultCollateralized();
    if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply);

    _yieldFeeTotalSupply -= _shares;
    _mint(_recipient, _shares);

    emit MintYieldFee(msg.sender, _recipient, _shares);
  }

This is an issue as anyone can steal the yield fee available by changing the _recipient input to any address. The different functions related to mintfee are all only callable by the the owner of the vault.

Tools Used

Manual review

Recommended Mitigation Steps

Use the onlyOwner modifier for mintYieldFee or change the function mintYieldFee(uint256 _shares, address _recipient) to only mintYieldFee(uint256 shares) and use as recipient the yieldFeeRecipient, that is never used.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The liquidate() and mintYieldFee() functions could leave the vaults undercollateralized.

Proof of Concept

_requireVaultCollateralized() is called at the beginning of mintYieldFee() and liquidate(). Or these two functions change the state, and the vault could become undercollateralized at the end of the functions.

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

function mintYieldFee(uint256 _shares, address _recipient) external {
    _requireVaultCollateralized();
    if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply);

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

  function mintYieldFee(uint256 _shares, address _recipient) external {
    _requireVaultCollateralized();
    if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply);

Tools Used

Manual review

Recommended Mitigation Steps

Call _requireVaultCollateralized() at the end of these functions instead of calling it at the beginning.

Assessed type

Invalid Validation