Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: low
Valid

I-01 — Informational

rubabbel

Root + Impact

Description

  • _isActivePlayer (src/PuppyRaffle.sol:173-180) is never used — dead code.

  • withdrawFees has no access control; funds can only go to feeAddress, so impact is limited, but it can be triggered by anyone.

  • enterRaffle accepts an empty array (msg.value == 0), a harmless no-op.

  • selectWinner emits no winner event, hurting off-chain indexing.

function _isActivePlayer() internal view returns (bool) { // @> never called
...
}
function withdrawFees() external { // @> no onlyOwner
...
}

Risk

Likelihood: N/A

Impact: Informational — code clarity and indexing only.

Proof of Concept

// No exploit; static observation. `grep -n "_isActivePlayer" src/PuppyRaffle.sol`
// shows only the definition, no call sites.

Recommended Mitigation

- function _isActivePlayer() internal view returns (bool) {
- for (uint256 i = 0; i < players.length; i++) {
- if (players[i] == msg.sender) {
- return true;
- }
- }
- return false;
- }
-
- function withdrawFees() external {
+ function withdrawFees() external onlyOwner nonReentrant {
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
+
+ // Emit a winner event for indexers
+ event WinnerSelected(address indexed winner, uint256 indexed tokenId, uint256 prize);

Reproducing all PoCs

forge test --match-contract ExploitTest -vv

Expected: 5/5 passing (testReentrancyRefund, testPredictableWinner,
testRefundBricksRaffle, testTotalFeesTruncation, testRefundDuplicatesBrickEntry).

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 42 minutes ago
Submission Judgement Published
Validated
Assigned finding tags:

[L-02] Missing `WinnerSelected`/`FeesWithdrawn` event emition in `PuppyRaffle::selectWinner`/`PuppyRaffle::withdrawFees` methods

## Description Events for critical state changes (e.g. owner and other critical parameters like a winner selection or the fees withdrawn) should be emitted for tracking this off-chain ## Recommendations Add a WinnerSelected event that takes as parameter the currentWinner and the minted token id and emit this event in `PuppyRaffle::selectWinner` right after the call to [`_safeMing_`](https://github.com/Cyfrin/2023-10-Puppy-Raffle/blob/07399f4d02520a2abf6f462c024842e495ca82e4/src/PuppyRaffle.sol#L153) Add a FeesWithdrawn event that takes as parameter the amount withdrawn and emit this event in `PuppyRaffle::withdrawFees` right at the end of [the method](https://github.com/Cyfrin/2023-10-Puppy-Raffle/blob/07399f4d02520a2abf6f462c024842e495ca82e4/src/PuppyRaffle.sol#L162)

Support

FAQs

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

Give us feedback!