First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

No comprehensive error handling for external calls, such as when sending funds to the winner.Which could lead to funds being stuck or lost in case of errors.

0xVinylDavyl

Summary

The contract does not include comprehensive error handling for external calls, such as when sending funds to the winner. This could lead to funds being stuck or lost in case of errors.

Vulnerability Details

The selectWinner function includes an external call to winner.call{value: prizePool}("") to send funds to the winner. However, there is no comprehensive error handling in case the external call fails for any reason, such as an out-of-gas condition or a failure in the recipient contract. Without proper error handling, funds may become inaccessible or lost.

In the original code, the selectWinner function includes an external call to send funds to the winner. Below is the relevant code block from the selectWinner function that demonstrates the external call:

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];
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);
}

The external call is performed with winner.call{value: prizePool}(""). This line sends the prizePool value to the winner address. However, there is no comprehensive error handling or fallback mechanism in place to handle potential failures in this external call, as indicated in the proof of concept.

Impact

External calls in Ethereum are inherently risky, as they can fail for various reasons, including out-of-gas, reverted calls, or invalid recipient addresses. Failing to handle these errors can result in stuck or lost funds, which is a critical issue for a smart contract.

The lack of error handling in external calls poses a significant risk of funds becoming inaccessible or lost. If the external call in the selectWinner function fails, it could lead to unintended financial consequences and negatively impact the contract's functionality.

Tools Used

Manual

Recommendations

To address this vulnerability, it is crucial to include proper error handling and fallback mechanisms for external calls to ensure the contract can respond appropriately to errors.

By adding proper error handling and fallback mechanisms for external calls, the contract can be enhanced in the sense that the contract's resilience to errors and ensure that funds are not lost in case of unexpected failures. This is a critical step in maintaining the reliability and security of the contract.

  1. Use a more robust method for external calls that can handle errors. For example, you can use the OpenZeppelin Address library's sendValue function, which includes built-in error handling.

  2. Implement checks to verify the success of the external call and handle failures gracefully. For example:

(bool success, ) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");

  1. Consider implementing a withdrawal pattern where the recipient (winner) can withdraw their funds at their convenience, reducing the risk of stuck funds in the contract.

Updates

Lead Judging Commences

hexbyte Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

smart contract wallet without fallback/receive will halt the raffle

Support

FAQs

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