Mystery Box

Summary

In both functions no checks-effects-interactions pattern is followed. If msg.sender is a contract and calls back into the contract via a fallback function or any other malicious logic, it could potentially cause a reentrancy attack, where the contract's balance is repeatedly drained in the same transaction.

Vulnerability Details

Both functions send Ether to msg.sender using .call, which forwards all remaining gas within this line of code

If msg.sender is a contract, it can invoke a reentrancy attack by calling back into this function before the rewardsOwned[msg.sender] array is deleted.

Impact

By reentering in both functions, a malicious user/contract could drain the whole balance of the contract.

Tools Used

Manual review

Recommendations

Use the checks-effects-interactions pattern to prevent reentrancy. Specifically, delete the rewards array before making the external call.

Alternatively, use reentrancy guard.