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

function refund() is vulnerable to a reentrancy attack

ETHANHUNTIMF99

Summary

The 'PuppyRaffle::refund' function() is vulnerable to a reentrancy attack, which can put the potential funds of the participants of the Raffle at risks.

Vulnerability Details

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 untrusted contract can then make a recursive call back to the original function, potentially leading to unexpected behavior or even draining of funds. Also there are no security checks for transferring the funds whether the transfer is success or not.

Impact

An attacker can deploy a malicious contract with a fallback function that can receive Ether and a function that can call the refund function of the "PuppyRaffle" contract.

The attacker calls the refund function, providing their own address as "playerAddress".

Inside the refund function, the "entrance fee" is sent back to the "attacker's" contract address using payable(msg.sender).sendValue(entranceFee).

The "attacker's" contract, having received the Ether, triggers the fallback function, which calls the refund function of your contract again, potentially leading to a "recursive loop of refund calls".

Since there is no check to prevent reentrancy, the attacker's contract can repeatedly call the refund function and easily steal the contract's balance.

Here is my proof of test code to show that the function is vulnerable to reentrancy attacks:

Firstly copy paste this test is your Truffle set up:

const PuppyRaffle = artifacts.require("PuppyRaffle");
const MaliciousContract = artifacts.require("MaliciousContract");
​
contract("PuppyRaffle Reentrancy Attack Test", (accounts) => {
let puppyRaffleInstance;
let maliciousContractInstance;
​
before(async () => {
puppyRaffleInstance = await PuppyRaffle.new(/* constructor arguments */);
maliciousContractInstance = await MaliciousContract.new(puppyRaffleInstance.address);
});
​
it("should demonstrate the reentrancy attack", async () => {
// Assuming you have a function in the malicious contract that triggers the attack
const attackResult = await maliciousContractInstance.triggerReentrancyAttack(/* parameters */);
// Add assertions to check the attack result or the state of the PuppyRaffle contract.
// For example, you can check the balance of the malicious contract to verify the attack.
const maliciousContractBalance = await web3.eth.getBalance(maliciousContractInstance.address);
assert.equal(maliciousContractBalance.toNumber(), expectedBalance, "Reentrancy attack was successful");
});
});

In the MaliciousContract.sol file, implement the malicious contract with a function that exploits the reentrancy vulnerability:

pragma solidity ^0.7.6;
​
contract MaliciousContract {
address public puppyRaffleAddress;
​
constructor(address _puppyRaffleAddress) {
puppyRaffleAddress = _puppyRaffleAddress;
}
​
function triggerReentrancyAttack(/* any necessary parameters */) public {
// Implement the reentrancy attack here, which repeatedly calls the refund function of PuppyRaffle.
// You'll need to use low-level calls or delegate calls to trigger the reentrancy.
}
}
​

Finally in the deploy_contracts.js migration script, make sure to deploy both the PuppyRaffle and MaliciousContract contracts:

const PuppyRaffle = artifacts.require("PuppyRaffle");
const MaliciousContract = artifacts.require("MaliciousContract");
​
module.exports = async (deployer) => {
await deployer.deploy(PuppyRaffle, /* constructor arguments */);
const puppyRaffleInstance = await PuppyRaffle.deployed();
​
await deployer.deploy(MaliciousContract, puppyRaffleInstance.address);
};
​

And then run the test, the test will demonstrate that the "refund function" in the PuppyRaffle contract can be easily exploited through a reentrancy attack by the malicious contract.

Tools Used

Remix IDE, Truffle

Recommendations

To mitigate this follow the "Checks-Effects-Interactions" pattern, where you perform all checks and state modifications before any external interactions

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); // Update player's status before any external interaction
​
// Transfer the entrance fee after the state change
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "PuppyRaffle: Refund failed");
​
emit RaffleRefunded(playerAddress);
}
​

This way, even if a reentrant call is made, the contract’s state will already be updated and the reentrant call will fail the require(playerAddress != address(0)) check .

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 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.