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

Reentrancy Vulnerability in `ChristmasDinner.sol::_refundERC20` and `ChristmasDinner.sol::_refundETH` functions leading to loss of funds.

poink

Description: When a user calls the ChristmasDinner.sol::refund function, it calls the internal functions _refundERC20 and _refundETH.
Both internal functions are vulnerable to reentrancy due to external calls made before state variables are updated.

In ChristmasDinner.sol::_refundERC20 tokens are transfered via safeTransfer() before balances[][] are updated to 0.

function _refundERC20(address _to) internal {
i_WETH.safeTransfer(_to, balances[_to][address(i_WETH)]);
i_WBTC.safeTransfer(_to, balances[_to][address(i_WBTC)]);
i_USDC.safeTransfer(_to, balances[_to][address(i_USDC)]);
balances[_to][address(i_USDC)] = 0;
balances[_to][address(i_WBTC)] = 0;
balances[_to][address(i_WETH)] = 0;
}

In ChristmasDinner.sol::_refundETH tokens are transfered via transfer() before etherBalance[] is updated to 0.

function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
_to.transfer(refundValue);
etherBalance[_to] = 0;
}

The ChristmasDinner.sol::nonReentrant modifier applied to the ChristmasDinner.sol::refund function does not protect against reentrancy due to the
locked variable always being set to false allowing functions with this modifier to be executed before previous execution is completed.

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

Impact: An attacker can refund to a malicious contract where the receive() function contains another call to to refund() leading to a
call loop where all funds are drained from the contract.

Recommended Mitigation: Reentrancy can be prevented if the ChristmasDinner.sol::nonReentrant modifier includes the following line:

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

Alternatively the vulnerable functions could implement the CEI pattern or Openzeppelin's ReentrancyGuard library could be used.

Updates

Lead Judging Commences

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

Give us feedback!