Christmas Dinner
First Flight #31
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Non-Conventional Implementation of the Non-Reentrant Modifier

nitzz

Summary

The nonReentrant modifier in the ChristmasDinner contract uses a less conventional implementation where the locked state variable is always reset to false at the end of the modifier, regardless of the function execution's success or failure. While the implementation prevents re-entrancy under normal circumstances, it deviates from the standard pattern, potentially leading to confusion or unforeseen edge cases.

Vulnerability Details

The nonReentrant modifier is implemented as follows:

modifier nonReentrant() {
require(!locked, "No re-entrancy");
_;
locked = false;
}
This pattern resets the locked variable to false only after the guarded function execution is complete. If the function fails or reverts midway, the locked variable may not return to its expected state, potentially locking the contract indefinitely. Although Solidity automatically reverts state changes on exceptions, this implementation is less robust and may confuse developers.

Impact

-> Potential Lockouts: In an edge case where execution is interrupted or fails, the locked flag may not reset correctly, rendering parts of the contract unusable.
-> Inconsistent Behavior: Fails to guarantee locked is properly reset when exceptions occur inside the guarded function.

Tools Used

manual review

Recommendations

update the nonReentrant modifier to follow the conventional implementation:

modifier nonReentrant() {
require(!locked, "No re-entrancy");
locked = true;
_;
locked = false;
}
Updates

Lead Judging Commences

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

mutex lock incomplete

Support

FAQs

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

Give us feedback!