Christmas Dinner

Summary

The ChristmasDinner::nonReentrant modifier has a logical flaw in updating the locked variable, rendering the reentrancy protection ineffective and exposing the contract to reentrancy attacks.

Vulnerability Details

The nonReentrant modifier is intended to prevent reentrancy attacks by using the locked state variable. However, there is a critical issue in its current implementation: the locked variable is only set to false after the function execution is complete, but it is not set to true at the start of the modifier's execution.

This means that during the execution of functions protected by the nonReentrant modifier, the locked state remains false. An attacker can recursively call the vulnerable function and bypass the require(!locked) check, thereby performing a reentrancy attack.

By repeatedly calling the ChristmasDinner::refund function, the attacker can exploit the fact that locked remains false, allowing the require(!locked) check to consistently pass, bypassing the reentrancy protection and draining the contract's funds.

Impact

This vulnerability can lead to severe financial loss. An attacker could repeatedly call the ChristmasDinner::refund function to withdraw funds multiple times, ultimately depleting the contract's balance. Such a reentrancy attack not only violates the contract's intended logic but also results in direct economic losses for users relying on the contract.

Tools Used

Manual Review

Recommendations

To fix this vulnerability and prevent reentrancy attacks, the nonReentrant modifier should be updated to set locked to true at the start of the function execution. Below is the corrected code: