Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

mint to the zero address

Summary

Whenever an pseudo empty slot == zero address array element at any index, selected as winner index by on-chain randomness login, mint to the zero address error exception occurs. This issue makes sense, according to EIP-721 NFT token can't be minted to zero address.

Vulnerability Details

select winner (zero address as winner)
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;
address winner = players[winnerIndex]; // here 👈, due to this line, this function raises the exception whenever zero address is announced as the winner and mints with this zero address at the end this function.
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
// statements...
// more statements...
_safeMint(winner, tokenId); // 👈 this statement raises the exception actually.
}

Impact

It's significantly crashes the Raffle.

Tools Used

Manual review.

Recommendations

We should recur the winner selection process until legit winner announcement.

address winner = address(0);
for (; winner == address(0);) {
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
winner = players[winnerIndex];
}

I'm not sure if this works mitigation works on mainnet (live chain), Since it raises MemoryLimitOOG exception on local testnet (foundry).

Or we should isolate all the legit players....

address[] private legitPlayers; // 👈 here first i added a state variable of address type array.
// statements...
// statements...
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
// here 👇 i implemented logic to isolate legit players.
for (uint256 i = 0; i < players.length; i++) {
if (players[i] == address(0)) {
console.log("yes found zero address");
continue;
}
console.log("zero address not found forwarding...");
legitPlayers.push(players[i]);
}
players = legitPlayers;
delete legitPlayers;
// 👆 my logic ended here above.
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
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);
}

and now we dont't need to modify uint256 totalAmountCollected = players.length * entranceFee; to uint256 totalAmountCollected = address(this).balance;

;`)

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

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

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!