PoolTogether - seeques's results

Lines of code

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

Vulnerability details

Impact

Anyone can pass an arbitrary recipient in mintYieldFee function to mint free shares.

Proof of Concept

When someone calls a liquidate function on a vault, the yieldFeeTotalSupply is increased if yieldFeePercentage != 0

    if (_yieldFeePercentage != 0) {
      _increaseYieldFeeBalance(
        (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut
      );
    }

This yield fee can then be withdrawn through a mintYieldFee function. The issue is that instead of transfering the yield fee to the _yieldFeeRecipient (a state variable), the function takes an arbitrary address as the recipient, meaning that anyone can mint free shares.

Tools Used

Manual review

Recommended Mitigation Steps

Transfer the yield fee to the _yieldFeeRecipient directly.

Assessed type

Access Control

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L570 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L498-L502 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L311-L330 (https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/abstract/TieredLiquidityDistributor.sol#L535-L536

Vulnerability details

Impact

It is possible to inflate the _reserve variable thus breaking the assumption stated in the contest's README file: The balance of prize tokens held by the contract must always be equal to the sum of the available tier liquidity and the reserve. Since _reserve is used as a cushion when there is insufficient tier liquidity for the prizes, the inflated reserves would not account for tier liquidity correctly. In case when there is indeed insufficient liquidity for the tier and normally accounted reserves are not enough to cover the deficiency (in this case the tx should revert), the inflated ones would "steal" liquidity from the next draws.

Proof of Concept

The prize tokens are contributed to the prizePool contract for the given vault by calling the contributePrizeTokens function on the prizePool contract. Users are incentivized to contribute POOL tokens in exchange for vault's shares by liquidating vault's yield in liquidate function which subsequently calls the contributePrizeTokens function for this vault. Users have to provide POOL tokens before liquidation. Generally, _reserves are increased as a portion of vault's contribution as per docs. It is possible to increase reserves manually by calling the increaseReserve function with the provided amount of POOL tokens which will be transfered from msg.sender account. As stated in README file, the balance of prize tokens in prize pool contract must always be equal to the sum of the available tier liquidity and the reserve. However, it is possible for liquidator to break this assumption by transfering prize tokens through the increaseReserve function just before liquidating the vault's yield. Such action would increase both the _reserves and the available tier liquidity for the next drawId.

A step by step actions (suppose the reserves and the tier liquidity are both zero and only one vault participates in a draw):

1. Liquidator calls increaseReserves function with amount of 1000 POOL tokens. _reserves == 1000, POOL token balance == 1000
2. Then he calls liquidate function on the vault (or it is possible to call contributePrizeTokens directly). totalAccumulator amount would be 1000.

Now with this share parameteres (only tokens to distribute would be 1000) and after all the prizes are distributed for all the tiers (assuming that no there is no remaining liquidity) the contract would have 70 POOL tokens in its balance and 1070 in its reserve.

It is also should be noted that if the draw manager decides to withdraw full reserve amount (accidentally or not), he will steal the liquidity from the next draw and also inflate the totalWithdrawn variable which will cause revert on every attempt to call contributePrizeTokens function because of underflow until someone transfers the withdrawn liquidity back.

Tools Used

Manual review

Recommended Mitigation Steps

I think it is best to add some kind of access restriction to increaseReserve function since normal users are not incentivized to call it anyway.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

Anyone can backrun the transaction in which the increaseReserve function is called to gain free liquidity for the vault by calling contributePrizeTokens.

Proof of Concept

If protocol admins (or someone esle) decide to increase reserves to provide a better incentive for a draw and a better cushion for tier liquidity in case it is insufficient, a malicious user who watches the mempool can backrun the transaction to gain free liquidity for the vault he is interested in by calling the contributePrizeTokens function. It is possible due to how contributePrizeTokens function works - it checks if a user transfered prize tokens before calling it - and due to the fact that reserves consist of prize pool tokens:

  function contributePrizeTokens(address _prizeVault, uint256 _amount) external returns (uint256) {
    uint256 _deltaBalance = prizeToken.balanceOf(address(this)) - _accountedBalance();
    if (_deltaBalance < _amount) {
      revert ContributionGTDeltaBalance(_amount, _deltaBalance);
    }

  function increaseReserve(uint104 _amount) external {
    _reserve += _amount;
    prizeToken.safeTransferFrom(msg.sender, address(this), _amount);
    emit IncreaseReserve(msg.sender, _amount);
  }

In addition, backrunning would not only benefit the vault with free liquidity but also inflate the reserve amount as I described in my previous finding.

Tools Used

Manual review

Recommended Mitigation Steps

The quickest way to fix this is to transfer prize tokens in time of execution the contributePrizeTokens function from msg.sender. However, the Vault's liquidate function should also be refactored in some way. Perhaps if the contribution comes from the liquidation, users should transfer prize tokens to vaults before calling liquidate from which they are taken afterwards by prizePool contract.

Assessed type

Timing