Puppy Raffle

AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: low
Likelihood: low
Invalid

Missing Array Bounds Check in refund()

Root + Impact

The refund() function does not explicitly check if playerIndex is within the bounds of the players array before accessing it. While Solidity automatically reverts on out-of-bounds access, the error message is generic and unhelpful, leading to poor user experience.

Description

Describe the normal behavior in one or more sentences:

  • The function should validate that the provided playerIndex is within the valid range of the players array

  • If an invalid index is provided, the function should revert with a clear, descriptive error message

Explain the specific issue or problem in one or more sentences:

  • The function accesses players[playerIndex] without first checking if playerIndex < players.length

  • When an out-of-bounds index is provided, Solidity reverts with a generic error message

  • Users receive unclear feedback about what went wrong, making debugging difficult

// @ src/PuppyRaffle.sol:96-105
function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex]; // No bounds check
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:
Reason 1: Only occurs with user error or malicious input providing invalid index
Reason 2: Solidity prevents actual security exploit through automatic bounds checking

Impact:
Impact 1: Poor error messaging confuses users when they provide invalid index
Impact 2: Slightly higher gas cost for failed transactions due to lack of early validation

Proof of Concept

Place the following test in test/AuditTest.t.sol:

function testOutOfBoundsRefund() public {
address[] memory players = new address[](1);
players[0] = playerOne;
puppyRaffle.enterRaffle{value: entranceFee}(players);
vm.prank(playerOne);
// Try to refund with invalid index
vm.expectRevert();
puppyRaffle.refund(999);
console.log("BUG CONFIRMED: Out of bounds access has unclear error message");
}

Run with: forge test --mt testOutOfBoundsRefund -vv

The test passes, showing the function reverts but with a generic error message.

Recommended Mitigation

Add explicit bounds check with clear error message:

function refund(uint256 playerIndex) public {
+ require(playerIndex < players.length, "PuppyRaffle: Invalid player index");
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");
payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!