52918 sc insight redundant check for allwinnersdrawn error
Submitted on Aug 14th 2025 at 09:45:35 UTC by @Finlooz4 for Attackathon | Plume Network
- Report ID: #52918
- Report Type: Smart Contract
- Report severity: Insight
- Target: https://github.com/immunefi-team/attackathon-plume-network/blob/main/plume/src/spin/Raffle.sol
Description
This report identifies a redundant error check in the requestWinner function: an explicit AllWinnersDrawn check duplicates behavior already enforced by the prize deactivation mechanism and thus is unnecessary.
Brief / Intro
The raffle contract contains a redundant AllWinnersDrawn error check in requestWinner. The same restriction is enforced by deactivating the prize once all winners have been drawn and by checks that prevent reactivation of completed prizes. The redundant check increases code complexity and consumes small amounts of additional gas.
Vulnerability Details
The requestWinner function contains a redundant AllWinnersDrawn check:
if (winnersDrawn[prizeId] >= prizes[prizeId].quantity) revert AllWinnersDrawn();
This is redundant because:
- When the last winner is selected,
handleWinnerSelectiondeactivates the prize:
if (winnersDrawn[prizeId] == prizes[prizeId].quantity) {
prizes[prizeId].isActive = false;
}
requestWinneralready checks the prize active status:
require(prizes[prizeId].isActive, "Prize not available");
setPrizeActiveprevents reactivation of completed prizes:
if (active) {
require(winnersDrawn[prizeId] < prize.quantity, "All winners already selected");
}
Because of the deactivation and reactivation prevention, attempting to request another winner after all winners are drawn will already revert via the isActive check. The explicit AllWinnersDrawn check therefore adds no additional protection.
The redundant check does not introduce a security vulnerability that allows incorrect behavior — it only increases gas usage and code complexity.
Impact Details
- Unnecessary condition check consumes extra gas.
- No change in functional behavior or security guarantees; both the redundant check and the active-check path correctly prevent additional winner requests.
Proof of Concept
Create a prize with quantity = 1
Create a prize whose quantity is 1.
Call requestWinner() and process VRF callback
winnersDrawnincrements to 1.- Prize deactivated (
isActive = false).
Attempt to call requestWinner() again
- With the redundant check: reverts with
AllWinnersDrawnerror. - Without the redundant check: reverts with "Prize not available".
Both outcomes correctly prevent additional winner requests
Both revert conditions prevent drawing another winner after the prize is completed.
Conclusion
The custom AllWinnersDrawn error provides no functional advantage over the existing active-status check and related reactivation prevention.
References
Related code snippets / links
- Target file: https://github.com/immunefi-team/attackathon-plume-network/blob/main/plume/src/spin/Raffle.sol
(See the functions and checks quoted above in the Vulnerability Details section.)