Submission Details
Severity:
Array of Players do not reduce when one player calls `refund()`
Summary
Array of Players do not reduce when one player calls refund()
Vulnerability Details
After entering the raffle, player can call refund() but the array of players don't reduce the size example : player[5] => players[4] so at the value at this index will be address(0)
Impact
Winner can be the player that called refund().
POC:
function testRefundPlayerButStillBeAddressZero() public {
address[] memory players = new address[](5);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
players[4] = address(5);
puppyRaffle.enterRaffle{value: entranceFee * 5}(players);
uint256 indexOfPlayer2 = puppyRaffle.getActivePlayerIndex(playerTwo);
vm.prank(playerTwo);
puppyRaffle.refund(indexOfPlayer2);
vm.warp(block.timestamp + duration + 2);
vm.roll(block.number + 1);
puppyRaffle.selectWinner();
}
RESULT:
[⠢] Compiling...
[⠆] Compiling 1 files with 0.7.6
[⠰] Solc 0.7.6 finished in 2.07s
Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[FAIL. Reason: ERC721: mint to the zero address] testRefundPlayerButStillBeAddressZero() (gas: 291661)
Traces:
[3322887] PuppyRaffleTest::setUp()
├─ [3258788] → new PuppyRaffle@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
│ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: PuppyRaffleTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← 12808 bytes of code
└─ ← ()
[271761] PuppyRaffleTest::testRefundPlayerButStillBeAddressZero()
├─ [148452] PuppyRaffle::enterRaffle{value: 5000000000000000000}([0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005])
│ ├─ emit RaffleEnter(newPlayers: [0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005])
│ └─ ← ()
├─ [1291] PuppyRaffle::getActivePlayerIndex(0x0000000000000000000000000000000000000002) [staticcall]
│ └─ ← 1
├─ [0] VM::prank(0x0000000000000000000000000000000000000002)
│ └─ ← ()
├─ [27458] PuppyRaffle::refund(1)
│ ├─ [60] PRECOMPILE::sha256{value: 1000000000000000000}()
│ │ └─ ← 0xe3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
│ ├─ emit RaffleRefunded(player: 0x0000000000000000000000000000000000000002)
│ └─ ← ()
├─ [0] VM::warp(86403 [8.64e4])
│ └─ ← ()
├─ [0] VM::roll(2)
│ └─ ← ()
├─ [60860] PuppyRaffle::selectWinner()
│ ├─ [0] 0x0000000000000000000000000000000000000000::fallback{value: 4000000000000000000}()
│ │ └─ ← ()
│ └─ ← "ERC721: mint to the zero address"
└─ ← "ERC721: mint to the zero address"
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.64ms
Tools Used
Foundry
Recommendations
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
+ require(players[winnerIndex] != address(0), "Winner is address 0");
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);
uint256 tokenId = totalSupply();
// We use a different RNG calculate from the winnerIndex to determine rarity
uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
}
POC:
function testRefundPlayerButStillBeAddressZero() public {
address[] memory players = new address[](5);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
players[4] = address(5);
puppyRaffle.enterRaffle{value: entranceFee * 5}(players);
uint256 indexOfPlayer2 = puppyRaffle.getActivePlayerIndex(playerTwo);
vm.prank(playerTwo);
puppyRaffle.refund(indexOfPlayer2);
vm.warp(block.timestamp + duration + 2);
vm.roll(block.number + 1);
+ vm.expectRevert("Winner is address 0");
puppyRaffle.selectWinner();
+ console.log("WINNER: ", puppyRaffle.previousWinner());
+ console.log("PLAYERS 2: ", puppyRaffle.players(indexOfPlayer2));
}
RESULT:
[⠢] Compiling...
[⠔] Compiling 3 files with 0.7.6
[⠒] Solc 0.7.6 finished in 2.20s
Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] testRefundPlayerButStillBeAddressZero() (gas: 210009)
Logs:
WINNER: 0x0000000000000000000000000000000000000000
PLAYERS 2: 0x0000000000000000000000000000000000000000
Traces:
[210009] PuppyRaffleTest::testRefundPlayerButStillBeAddressZero()
├─ [148452] PuppyRaffle::enterRaffle{value: 5000000000000000000}([0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005])
│ ├─ emit RaffleEnter(newPlayers: [0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005])
│ └─ ← ()
├─ [1291] PuppyRaffle::getActivePlayerIndex(0x0000000000000000000000000000000000000002) [staticcall]
│ └─ ← 1
├─ [0] VM::prank(0x0000000000000000000000000000000000000002)
│ └─ ← ()
├─ [27458] PuppyRaffle::refund(1)
│ ├─ [60] PRECOMPILE::sha256{value: 1000000000000000000}()
│ │ └─ ← 0xe3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
│ ├─ emit RaffleRefunded(player: 0x0000000000000000000000000000000000000002)
│ └─ ← ()
├─ [0] VM::warp(86403 [8.64e4])
│ └─ ← ()
├─ [0] VM::roll(2)
│ └─ ← ()
├─ [0] VM::expectRevert(Winner is address 0)
│ └─ ← ()
├─ [5364] PuppyRaffle::selectWinner()
│ └─ ← "Winner is address 0"
├─ [2448] PuppyRaffle::previousWinner() [staticcall]
│ └─ ← 0x0000000000000000000000000000000000000000
├─ [0] console::log(WINNER: , 0x0000000000000000000000000000000000000000) [staticcall]
│ └─ ← ()
├─ [702] PuppyRaffle::players(1) [staticcall]
│ └─ ← 0x0000000000000000000000000000000000000000
├─ [0] console::log(PLAYERS 2: , 0x0000000000000000000000000000000000000000) [staticcall]
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.71ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Updates
Lead Judging Commences
Hamiltonite almost 2 years ago
Submission Judgement Published Assigned finding tags:
refund-doesnt-reduce-players-array-size-causing-protocol-to-freeze
zero address can win the raffle
Funds are locked to no one. If someone gets the refund issue, they also got this issue. IMPACT: High Likelihood: High
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.