First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

The refund function is susceptible to reentrancy

happyformerlawyer

Summary

The refund function does not follow the pattern of checks, effects, interactions, Instead it sends the refund before making state changes. Also, the OpenZeppelin docs specifically note that you are handing control over to msg.sender when you use their sendValue function. This is an unsafe external call that would allow an attacker to drain the contract.

Vulnerability Details

function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
​
payable(msg.sender).sendValue(entranceFee);
​
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Impact

Someone could write a contract that would repeatedly call into refund and drain the contract of an amount equal to the number of calls times the entrance fee.

Tools Used

Manual review

Recommendations

Move "payable(msg.sender).sendValue(entranceFee)" to the end of the the refund function, after the state changes.

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

reentrancy-in-refund

reentrancy in refund() function

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!