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

Refund is Vulnerable to Reentrancy Attack

kose

Summary

Player can get all the balance from the contract via reentrancy in refund.

Vulnerability Details

Refund function does not follow CEI pattern and also don't have ReentrancyGuard. In refund() control is transferred to recipient via sendValue function from Address library as shown below:

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); // HIGH - Reentrancy
​
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

And the playerIndex is removed from players array only after recipient give the control back to the contract. But sendValue function does a low level call as shown below:

function sendValue(address payable recipient, uint256 amount) internal {
require(address(this).balance >= amount, "Address: insufficient balance");
​
// solhint-disable-next-line avoid-low-level-calls, avoid-call-value
(bool success, ) = recipient.call{ value: amount }("");
require(success, "Address: unable to send value, recipient may have reverted");
}

Hence during this call, recipient might be malicious and via fallback function it can call the return function again with the same playerIndex. Since playerIndex is still in players array, all require statements will be success and player will continue to get entranceFee from protocol until all balance zeroed out.

Impact

Stealing all funds from contract, hence high.

Tools Used

Manual Review

Recommendations

Always follow CEI pattern and/or use ReentrancyGuard modifier.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 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.