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

Incorrect implementation of nonReentrant modifier allows to drain the contract

al88nsk

Summary

Modifier ChristmasDinner::nonReentrant does not set the locked variable to true allowing to reenter the functions protected by nonReentrant modifier.

Vulnerability Details

Modifier nonReentrant is implemented as below:

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

The modifier relies on the locked variable to lock the contract, but it never sets locked to true.

Impact

Function ChristmasDinner::_refundETH does not follow CEI pattern, but it relies on the nonReentrant modifier to protect from reentrancy attacks. Since the nonReentrant is implemented incorrectly and it does not set the locked variable to true, this protection does not work and allows to drain the contract eth balance.

Tools Used

Manual review

Recommendations

Set locked variable to true after checking its value as below:

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

Lead Judging Commences

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