Mystery Box

Summary

The claimAllRewards() function is vulnerable to a reentrancy attack. This occurs because external calls are made before the contract state is updated, allowing an attacker to repeatedly call the function in a fallback, potentially draining the contract of its funds.

Vulnerability Details

The function transfers funds to the user by calling payable(msg.sender).call{value: totalValue}(""); before the state variable rewardsOwned is updated. This gives an attacker the opportunity to exploit the contract by invoking a fallback function and calling claimAllRewards() again before the deletion of rewards takes place.

An attacker can use this vulnerability to repeatedly call the claimAllRewards() function in their fallback function, draining all funds from the contract or claiming more rewards than they are entitled to.

The function does not follow the Check-Effects-Interactions pattern, which recommends updating the contract’s state before making external calls

• Vulnerable Code Snippet

Impact

An attacker could exploit this vulnerability to drain all funds from the contract, claim multiple rewards, or cause a severe financial loss. This could lead to significant damage to the integrity of the contract and the trust of users interacting with it.

All users of the contract are at risk since an attacker could abuse this reentrancy vulnerability and manipulate reward claims, rendering the contract unusable or depleting its funds.

Tools Used

Manual Review

Recommendations

Consider using the Check-Effects-Interactions pattern by moving the deletion of the rewardsOwned[msg.sender] state variable to occur before the external call to the msg.sender. This ensures the contract’s state is updated before any interactions with external addresses take place.

As I have shown here :