Summary
Methods used to generate randomness is not truly random and can be manipulated by an attacker.
CEI pattern violations making the contract vulnerable to reentrancy attacks.
Technical debts leading to increased deployment cost.
Using solidity require statements for reverts making the contract 'gas bad'.
Vulnerability Details
It is possible to exploit the raffle to draw a particular address as a winner as the method used to generate randomness is not truly random and can be manipulated by an attacker.
It is possible to exploit the raffle to draw a particular puppy as the method used to generate randomness is not truly random and can be manipulated by an attacker.
CEI pattern violation in solidity enterRaffle method making it vulnerable to reentrancy attacks.
CEI pattern violation in solidity refund method making it vulnerable to reentrancy attacks.
CEI pattern violation in solidity selectWinner method making it vulnerable to reentrancy attacks.
solidity _isActivePlayer method is an internal function not being used anywhere, thus is technical debt.
solidity _baseURI returns a constant, thus is technical debt.
Using solidity require statements for reverts make the contract less gas efficient for both users and the protocol.
Impact
Attacker can exploit the protocol to win the raffle.
Attacker can exploit the protocol to win a particular puppy.
Contract is vulnerable to reentrancy attacks.
High deployment cost.
Not gas efficient for users.
Tools Used
Foundry
Chainlink
OpenZeppelin
Recommendations
contract PuppyRaffle is ERC721, VRFConsumerBaseV2, ReentrancyGuard, Ownable {...}
function selectWinner() external nonReentrant returns (uint256 request_id) {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
request_id = i_vrfCoordinator.requestRandomWords(
i_gasLane, i_subscriptionId, REQUEST_CONFIRMATIONS, i_callbackGasLimit, NUM_WORDS
);
}
function fulfillRandomWords(uint256, uint256[] memory randomWords) internal override nonReentrant {
uint256 winnerIndex = randomWords[0] % 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 rarity = randomWords[1] % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[totalSupply] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[totalSupply] = RARE_RARITY;
} else {
tokenIdToRarity[totalSupply] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
_safeMint(winner, totalSupply);
totalSupply += 1;
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
}
contract PuppyRaffle is ERC721, VRFConsumerBaseV2, ReentrancyGuard, Ownable {...}
function enterRaffle(address[] memory newPlayers) public payable nonReentrant {
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
for (uint256 i = 0; i < newPlayers.length; i++) {
players.push(newPlayers[i]);
}
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}
emit RaffleEnter(newPlayers);
}
function refund(uint256 playerIndex) public nonReentrant {
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);
}
function selectWinner() external nonReentrant returns (uint256 request_id) {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
request_id = i_vrfCoordinator.requestRandomWords(
i_gasLane, i_subscriptionId, REQUEST_CONFIRMATIONS, i_callbackGasLimit, NUM_WORDS
);
}
function fulfillRandomWords(uint256, uint256[] memory randomWords) internal override nonReentrant {
uint256 winnerIndex = randomWords[0] % 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 rarity = randomWords[1] % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[totalSupply] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[totalSupply] = RARE_RARITY;
} else {
tokenIdToRarity[totalSupply] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
_safeMint(winner, totalSupply);
totalSupply += 1;
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
}
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
require(_exists(tokenId), "PuppyRaffle: URI query for nonexistent token");
uint256 rarity = tokenIdToRarity[tokenId];
string memory imageURI = rarityToUri[rarity];
string memory rareName = rarityToName[rarity];
return string(
abi.encodePacked(
"data:application/json;base64,",
Base64.encode(
bytes(
abi.encodePacked(
'{"name":"',
name(),
'", "description":"An adorable puppy!", ',
'"attributes": [{"trait_type": "rarity", "value": ',
rareName,
'}], "image":"',
imageURI,
'"}'
)
)
)
)
);
}
error PuppyRaffle__NotEnoughValue();
error PuppyRaffle__DuplicatePlayer();
error PuppyRaffle__OnlyPlayerCanRefund();
error PuppyRaffle__PlayerAlreadyRefundedOrNotActive();
error PuppyRaffle__RaffleNotOver();
error PuppyRaffle__NotEnoughPlayers();
error PuppyRaffle__FailToSelectWinnerFailToSendPrize();
error PuppyRaffle__FailToWithdrawNotEnoughPlayers();
error PuppyRaffle__FailToWithdrawFailToSendEther();
Example custom error,
if (msg.value != entranceFee * newPlayers.length) {
revert PuppyRaffle__NotEnoughValue();
}
