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

Refund Function Lacks Balance Verification Leading to Failed Transfers

0xmaxi

Summary

The refund() function attempts to process token and ETH transfers without first verifying if the user has any balance to refund, leading to unnecessary gas consumption and potential failed transactions.

Vulnerability Details

The current implementation blindly attempts to transfer tokens and ETH regardless of whether the user has any balance:

function refund() external nonReentrant beforeDeadline {
address payable to = payable(msg.sender);
refundERC20(_to);
refundETH(_to);
emit Refunded(msg.sender);
}

The internal functions _refundERC20() and _refundETH() will attempt transfers even when balances are zero:

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)]);
// ...
}
function refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[to];
to.transfer(refundValue);
// ...
}

Impact

While this isn't a direct security vulnerability, it has several negative implications:

  1. Unnecessary gas consumption for users attempting to refund with zero balances

  2. Potential for failed transactions due to zero-value transfers

  3. Misleading event emissions suggesting successful refunds when no actual value was transferred

  4. Poor user experience and increased transaction costs

Tools Used

  • Manual review

Recommendations

Add balance checks before processing refunds:

function refund() external nonReentrant beforeDeadline {
address payable to = payable(msg.sender);
// Check if user has any balance to refund
bool hasERC20Balance = (
balances[to][address(i_WETH)] > 0 ||
balances[to][address(i_WBTC)] > 0 ||
balances[to][address(i_USDC)] > 0
);
bool hasETHBalance = etherBalance[to] > 0;
if (!hasERC20Balance && !hasETHBalance) {
revert NoBalanceToRefund();
}
if (hasERC20Balance) {
refundERC20(_to);
}
if (hasETHBalance) {
refundETH(_to);
}
emit Refunded(msg.sender);
}

Additional recommendations:

  1. Add a custom error NoBalanceToRefund()

  2. Consider emitting more detailed events that specify which tokens were refunded and their amounts

  3. Consider implementing individual token refund functions to give users more control

Updates

Lead Judging Commences

0xtimefliez Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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