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

Reentrancy in refund can drain funds

aballok

Summary

Checks Effect Interactions not applied in refund can lead to reentrancy

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);
}

An external call is made to an address payable(msg.sender).sendValue(entranceFee) and if that address is a contract on receiving ETH can reenter the contract by calling refund and claim more refund as state is only updated after the external call.

sendValue forwards all gas unlike .transfer and .send which limit to 2300 so sendValue() is susceptible to reentrancy

Impact

Reentrancy can lead to draining and stealing of contract and user funds in raffle contract

Tools Used

Manual Analysis

Recommendations

Ensure CEI patter in followed e,g update state first before external calls

players[playerIndex] = address(0);
payable(msg.sender).sendValue(entranceFee);

And or apply some nonReentrant modifer or protection mechanisms e.g make use of OpenZeppelin Reentrancy contracts and apply nonReentrant to function

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!