Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Incorrect Calculation of `totalAmountCollected` in the `selectWinner` Function

Summary

In the selectWinner function of the PuppyRaffle smart contract, the totalAmountCollected is calculated by multiplying the entranceFee with the length of the participants array. However, the participants array is not updated after the refund function is called, and any ether sent to the contract is added to the totalAmountCollected. This incorrect calculation might lead to the totalAmountCollected being higher than the actual amount of ether in the contract, potentially allowing an attacker to drain the owner's fees (totalFees).

Vulnerability Details

The vulnerability arises from the inaccurate calculation of totalAmountCollected. Specifically:

  1. The selectWinner function calculates totalAmountCollected by multiplying the entranceFee with the length of the participants array.

  2. When a user calls the refund function, address in that index of the participants array is set to address(0), but the array length is not updated. This means that the number of participants will remain the same, even after a user is refunded.

  3. The attacker can pre calculate the totalAmountCollected where the 80% share is equal to contract balance and transfer the money to the winner leaving the owner with nothing to collect.

Impact

  • High: An attacker may exploit this vulnerability to drain the owner's fees (totalFees), potentially causing significant financial loss to the contract.

Tools Used

  • Manual review of the smart contract code.

Recommendations

To address the incorrect calculation of totalAmountCollected and mitigate the associated risk, consider the following recommendations:

  1. Calculate totalAmountCollected Based on Contract Balance: Instead of relying on the length of the participants array, calculate totalAmountCollected by checking the balance of the contract. This approach ensures that the calculation is accurate and reflects the actual funds available in the contract.

  2. Use a Mapping to Store Participants: Consider using a mapping to store participants' addresses and a counter to keep track of the number of participants. When the refund function is called, remove the participant from the mapping and decrease the counter by 1. This will provide a more efficient and accurate way to manage participants.
    OR Update participants Array After Refunds: Alternatively, you can update the participants array to remove refunded participants, maintaining an accurate representation of active participants.

Updates

Lead Judging Commences

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

refund-doesnt-reduce-players-array-size-causing-protocol-to-freeze

zero address can win the raffle

Funds are locked to no one. If someone gets the refund issue, they also got this issue. IMPACT: High Likelihood: High

Support

FAQs

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