PoolTogether - qpzm's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/55dcd65de4436d50967819c2ac313c3910b6f5f3/src/Vault.sol#L480-L482 https://github.com/generationsoftware/pt-v5-twab-controller/blob/1cdf78e87a3d9127f85a3755024f143664643c5e/src/TwabController.sol#L500-L502 https://github.com/generationsoftware/pt-v5-twab-controller/blob/1cdf78e87a3d9127f85a3755024f143664643c5e/src/TwabController.sol#L656-L661 https://github.com/generationsoftware/pt-v5-twab-controller/blob/1cdf78e87a3d9127f85a3755024f143664643c5e/src/TwabController.sol#L24

Vulnerability details

Impact

TwabController.delegateBalance is related to the probability to get the prize, and Vault.sponsor can make the others' delegateBalance to 0. A malicious user can send a small amount of assets to every depositor and be the only prize taker.

Proof of Concept

Paste this test in VaultDepositTest below this line. https://github.com/GenerationSoftware/pt-v5-vault/blob/55dcd65de4436d50967819c2ac313c3910b6f5f3/test/unit/Vault/Deposit.t.sol#L401 I commented the purpose of the code.

function testSponsor_AliceTakesAwayPrizeChanceFromBob() external {
    // @audit bob deposits $5000
    vm.startPrank(bob);

    uint256 _amount1 = 5000e18;
    underlyingAsset.mint(bob, _amount1);
    underlyingAsset.approve(address(vault), type(uint256).max);
    vault.deposit(_amount1, bob);

    assertEq(vault.balanceOf(bob), _amount1);
    assertEq(twabController.delegateBalanceOf(address(vault), bob), _amount1);

    vm.stopPrank();

    // @audit alice sponsors bob with little assets
    vm.startPrank(alice);
    uint256 _amount2 = 1;
    underlyingAsset.mint(alice, _amount2);
    underlyingAsset.approve(address(vault), type(uint256).max);

    vault.sponsor(_amount2, bob);
    vm.stopPrank();

    // @audit bob has no chance to get the prize. This should not pass.
    assertEq(twabController.delegateBalanceOf(address(vault), bob), 0);
}

Then run these commands.

cd vault
forge test --match-test testSponsor_AliceTakesAwayPrizeChanceFromBob

This changes the probability of getting the prize, because TwabLib.AccountDetails.delegateBalance is the actual balance recorded in the TwabLib.Account.observations mapping. https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/1cdf78e87a3d9127f85a3755024f143664643c5e/src/libraries/TwabLib.sol#L203 https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/1cdf78e87a3d9127f85a3755024f143664643c5e/src/libraries/TwabLib.sol#L119

The observations mapping is used in PrizePool._isWinner to calculate the winning zone based on _userTwab. https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L864 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L928 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L93

Tools Used

Manual review

Recommended Mitigation Steps

TwabController.sponsor had better take an additional input argument uint256 amount to indicate the amount to sponsor.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

On every withdraw/redeem, assets in the vault will decrease faster than expected and finally, the vault will drain.

Proof of Concept

Vault._withdraw assumes _yieldVault.withdraw(_assets, address(this), address(this)); will return the same amount of tokens as _assets but this is not the case if _yieldVault is a fee-on-transfer token. https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1026-L1027

Tools Used

Manual review

Recommended Mitigation Steps

Calculate the difference of the asset and transfer the amount to the receiver.

+ uint256 assetBefore = IERC20(asset()).balanceOf(address(this)); 
_yieldVault.withdraw(_assets, address(this), address(this));
+ uint256 assetAfter = IERC20(asset()).balanceOf(address(this));
SafeERC20.safeTransfer(IERC20(asset()), _receiver, assetAfter - assetBefore);

Assessed type

Token-Transfer