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

`PuppyRaffle::refund` allows a malicious player to drain funds

0xrex

Summary

The refund function can be exploited by the .sendValue external call to continue withdrawing the entranceFee until the PuppyRaffle contract's balance is drained in a reentrancy attack.

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); // vulnerable line of code
​
@> players[playerIndex] = address(0); // vulnerability exposed line of code
emit RaffleRefunded(playerAddress);
}

Impact

When I call the refund function, I supply my valid playerIndex number the function uses to find my address in the players array of addresses then proceeds to send me the public global entranceFee.

However, I can just deploy a smart contract to run a drain attack logic in the receive or fallback function that enables me to keep calling the PuppyRaffle::refund function as long as the balance is greater than a certain amount, e.g the entrance fee.

Tools Used

Manual review

Recommendations

The fix for this is to either implement a nonReentrant lock or opt for the CEI pattern by clearing my address from the players array before making the transfer like so:

function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex]; // our index exists √
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund"); // it's us calling this function √
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active"); // our address is not 0 √
+ players[playerIndex] = address(0);
payable(msg.sender).sendValue(entranceFee);
- players[playerIndex] = address(0);
​
emit RaffleRefunded(playerAddress);
}
​
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.