Mystery Box

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

Updating state after external call in `MysteryBox::claimSingleReward` leads to a Re-entrancy vulnerability.

Summary

The function claimSingleReward(uint256 _index) in the MysteryBox contract is vulnerable to a re-entrancy attack. This vulnerability arises because the function makes an external call to transfer funds to the user with payable(msg.sender).call before updating the contract’s state by deleting the reward at the specified index delete rewardsOwned[msg.sender][_index]. This allows an attacker to re-enter the function and claim the reward multiple times before the state is updated.

Vulnerability Details

An attacker could deploy a malicious contract to exploit this vulnerability:

  • The attacker’s contract calls claimSingleReward(\_index).

  • The external call to payable(msg.sender).call{value: value}("") is made before the state is updated delete rewardsOwned[msg.sender][_index].

  • The attacker’s contract re-enters claimSingleReward(_index) before the state is updated, allowing the attacker to claim the reward multiple times within the same transaction.

  • This cycle repeats, leading to the attacker draining all funds associated with the reward at the specified index.

Impact

An attacker can exploit this vulnerability by repeatedly calling claimSingleReward() before the contract’s state is updated. The attacker can drain all funds associated with the specified index, taking advantage of the re-entrancy to call the function multiple times in the same transaction, effectively draining funds from the contract.

Tools Used

Manual Review, unit-testing.

Recommendations

  1. Carefully implement the checks->effects->interactions pattern throughout MysteryBox . In particular, make sure that it is not being violated within function calls. This ensures that all external calls are made after state updates.

  2. Use a reentrancy guard, the best known of which is OpenZeppelin’s ReentrancyGuard, and apply its modifier nonReentrant() on all public and external functions.

Updates

Appeal created

inallhonesty Lead Judge 8 months 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.