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

In the `ChristmasDinner` contract the `nonReentrant` modifier is functionally broken

phylax

Summary

The ChristmasDinner::nonReentrant does not set the mutex lock to true when a function is called making it possible to do an reentrancy attack:

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

Vulnerability Details

When the refund function is called it would be possible to reenter even though it uses the nonReentrantmodifier. An reentrancy attack would still not be possible because the _refundETHfunction uses transferinstead of low level call so the transaction would run out of gas before an attacker could reenter. Still the modifier is not functioning as it should and the natspec is incorrect.

/**
@> * @dev ERC20 withdrawal of all user funds. No concern for Reentrancy
@> * since refund() uses a Mutex Lock
* @param _to payable address passed from refund()
*/
function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
_to.transfer(refundValue);
etherBalance[_to] = 0;
}

Tools Used

Manuel code review

Recommendations

Add this line to the modifier:

modifier nonReentrant() {
require(!locked, "No re-entrancy");
// should add the following line to prevent reentrancy
+ 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!