Mystery Box
First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

reentrancy attack which can lead to drain of funds

cody

Summary

Vulnerability Details

https://github.com/Cyfrin/2024-09-mystery-box/blob/main/src/MysteryBox.sol#L92

claimSingleReward allows a user (i.e., msg.sender) to claim a specific reward from their list of rewards, the isue here is that The Ether transfer occurs before the state (rewardsOwned[msg.sender][_index]) is updated, which could leave this function vulnerable to a reentrancy attack.

Impact

A malicious contract could re-enter the function before the reward is deleted, claiming rewards multiple times

Tools Used

manual review

Recommendations

The line delete rewardsOwned[msg.sender][_index]; is moved before the Ether is transferred using .call.

+ delete rewardsOwned[msg.sender][_index];
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
​
- delete rewardsOwned[msg.sender][_index];
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`claimSingleReward` reentrancy

Support

FAQs

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