code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Claim failure can result to loss of funds #390

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L407-L456

Vulnerability details

Impact

Caller will lose funds through gas fees when claim fails

Proof of Concept

The claimPrize function allows any caller to claim a prize by providing the necessary parameters: _winner (address of the winner), _tier (prize tier), _prizeIndex (prize index), _prizeRecipient (address to receive the prize), _fee (claim fee), and _feeRecipient (address to receive the fee). However, the function does not verify that the caller (msg.sender) is actually the winner of the specified prize. It relies on the _isWinner internal function to check whether the provided _winner, _tier, and _prizeIndex match the winning criteria. If _isWinner returns true, the prize is claimed. The vulnerability arises from the fact that _isWinner relies solely on the provided parameters without validating the caller's identity. This means that anyone can call the claimPrize function and provide arbitrary _winner, _tier, and _prizeIndex values. However the contract will revert with a DidNotWin error leading to loss of funds through gas fees

Tools Used

Manual

Recommended Mitigation Steps

Add an additional check at the beginning of theclaimPrize function to ensure that the caller is the actual winner of the prize they are claiming. This can be done by comparing msg.sender with the _winner parameter and reverting if they don't match.

Assessed type

Token-Transfer

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid