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
).
The vulnerability arises from the inaccurate calculation of totalAmountCollected
. Specifically:
The selectWinner
function calculates totalAmountCollected
by multiplying the entranceFee
with the length of the participants
array.
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.
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.
High: An attacker may exploit this vulnerability to drain the owner's fees (totalFees
), potentially causing significant financial loss to the contract.
Manual review of the smart contract code.
To address the incorrect calculation of totalAmountCollected
and mitigate the associated risk, consider the following recommendations:
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.
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.
Funds are locked to no one. If someone gets the refund issue, they also got this issue. IMPACT: High Likelihood: High
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.