sherlock-audit / 2024-05-pooltogether-judging

9 stars 5 forks source link

snapishere - DOS of claimprize function in the PrizePool.sol+ Loss of Tokens #35

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

snapishere

medium

DOS of claimprize function in the PrizePool.sol+ Loss of Tokens

Summary

DOS of claimprize function in the PrizePool.sol

Vulnerability Detail

claimPrize function cant be called without difference in _totalAccumulator variable and DONATOR vaults accumulator Front run can causes further DOS of the claimPrize function and loss of users prizeTokens.

Impact

Code Snippet

image

The totalSupply being equal to 0 causes the exploit.

image

Revert caused on this line https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-prize-pool/src/PrizePool.sol#L1116

Unless this function is called with a _priceVault not equal to the DONATOR nobody will be able to call the claim Prize function in this contract but as there is no safe way to call this without being front run its left vulnerable to DOS.

Not only that the function causes the loss of _prizeTokens meaning the person contributing will either have to get or mint more of _prizeTokens to the contract possibly causing depreciation in _prizeTokens value. example test of exploit:

function testfrontrunexploitpreventingclaimprize() public {
    prizeToken.mint(address(prizePool), 100e18);
    vm.prank(address(1));
    prizePool.contributePrizeTokens(prizePool.DONATOR(), 100e18);
    vm.stopPrank();
    vm.expectRevert();
    prizePool.contributePrizeTokens(address(this), 100e18);
    awardDraw(1);
    mockTwab(address(this), msg.sender, 0);
    vm.expectRevert();
    uint256 prize = claimPrize(msg.sender, 0, 0);
    assertEq(prizePool.accountedBalance(), 100e18 - prize);
 }

Tool used

Manual Review

Recommendation

replace the contributePrizeToken function with

function donatePrizeTokens(uint256 _amount, address _prizeVault) external { prizeToken.safeTransferFrom(msg.sender, address(this), _amount); contributePrizeTokens(_prizeVault, _amount); } Stopping the front running. If you do not want to transfer no tokens to a different vault to call claimPrize you should make a case for the totalContributed in here: image

If totalContributed is 0 which causes the divide by 0 error in the first place.

nevillehuang commented 3 months ago

Invalid, division by zero not possible due to check here