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

Unsafe ETH Transfer Without Address Validation

mishraji874

Description: The refund() function in the ChristmasDinner contract sends ETH to users without performing proper address validation checks. This is particularly concerning in the internal _refundETH function that's called by refund(). The actual vulnerability will be found here in the contract:

function refund() external nonReentrant beforeDeadline {
address payable _to = payable(msg.sender);
_refundERC20(_to);
_refundETH(_to);
emit Refunded(msg.sender);
}
​
function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
_to.transfer(refundValue);
etherBalance[_to] = 0;
}

Impact:

  • Potential Loss of Funds:

    1. If msg.sender is a contract address that can't handle ETH (no receive/fallback function)

    2. If msg.sender is an invalid or incorrectly formatted address

    3. If the address is self-destructed between balance check and transfer

  • Failed Transactions:

    1. Transfer to invalid addresses will cause the entire transaction to revert

    2. This could block legitimate refund attempts

  • Smart Contract Integration Issues:

    1. Contracts interacting with this function might fail unexpectedly

    2. No way to handle failed transfers gracefully

Proof of Concept:

// Test contract demonstrating the vulnerability
contract VulnerabilityTest {
ChristmasDinner dinner;
constructor(address _dinner) {
dinner = ChristmasDinner(_dinner);
}
// This contract has no receive() or fallback()
function depositAndRefund() external payable {
// Deposit ETH
(bool success,) = address(dinner).call{value: msg.value}("");
require(success, "Deposit failed");
// Attempt refund - This will fail because contract can't receive ETH
dinner.refund();
}
}
​
// Test script
function testFailedRefund() public {
// Deploy vulnerable contract
ChristmasDinner dinner = new ChristmasDinner(wbtc, weth, usdc);
// Deploy test contract
VulnerabilityTest test = new VulnerabilityTest(address(dinner));
// Attempt deposit and refund
vm.expectRevert(); // Expected to revert
test.depositAndRefund{value: 1 ether}();
}

Recommended Mitigation:

  1. Add Address Validation:

function refund() external nonReentrant beforeDeadline {
address payable _to = payable(msg.sender);
require(_to != address(0), "Invalid address");
require(_to != address(this), "Cannot refund to contract");
_refundERC20(_to);
_refundETH(_to);
emit Refunded(msg.sender);
}

  1. Implement Safe Transfer Pattern:

function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
// Clear balance before transfer to prevent reentrancy
etherBalance[_to] = 0;
// Use low-level call with gas stipend
(bool success, ) = _to.call{value: refundValue, gas: 2300}("");
require(success, "ETH transfer failed");
emit ETHRefunded(_to, refundValue);
}

  1. Add Try-Catch Pattern:

function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
etherBalance[_to] = 0;
try _to.call{value: refundValue, gas: 2300}("") returns (bool success) {
require(success, "ETH transfer failed");
emit ETHRefunded(_to, refundValue);
} catch {
// Revert the balance change if transfer fails
etherBalance[_to] = refundValue;
emit RefundFailed(_to, refundValue);
revert("ETH transfer failed");
}
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

transfer instead of call

Support

FAQs

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