Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium

Unhandled Ether Transfer

0xVinylDavyl

Summary

Tests assert that the Ether balances of the proxies remain at zero, but the contracts may receive Ether due to the lack of proper handling.

Vulnerability Details

The "ProxyTest" contract contains a test function, "testIfSendEtherToProxyThenRevert," where Ether is sent to instances of the "Proxy" contract using the .call{value: ...}("") syntax. However, the test cases do not properly handle the revert condition that should occur when trying to send Ether to a contract that does not have a payable fallback function. As a result, the tests assert that the Ether balances of the proxies remain at zero, but the contracts may receive Ether due to the lack of proper handling.

function testIfSendEtherToProxyThenRevert() public {
vm.deal(msg.sender, 2 ether);
vm.expectRevert();
(bool success,) = address(proxy).call{value: 1 ether}("");
// no ether arrived
assertEq(0, address(proxy).balance);
​
(bool success2,) = address(secondProxy).call{value: 1 ether}("");
// no ether arrived
assertEq(0, address(secondProxy).balance);
}
​

Impact

The lack of proper handling of Ether transfers to contracts that do not have a payable fallback function can lead to unintended behavior and security vulnerabilities. This could result in loss of Ether sent to the contracts, as well as potential disruption of the contracts' functionality.

Tools Used

Manual

Recommendations

  1. Update the test cases to include assertions that check for the proper handling of Ether transfers to the proxy contracts that lack a payable fallback function.

  2. Use try-catch or require statements to handle the revert condition and verify that Ether transfers to non-payable contracts are properly rejected.
    By addressing this vulnerability and ensuring proper handling of Ether transfers, you can prevent unintended Ether transfers and potential disruptions to contract behavior.

function testIfSendEtherToProxyThenRevert() public {
// Attempt to send Ether to the proxy contract without a payable fallback
try address(proxy).call{value: 1 ether}("") {
// Revert the test and fail if Ether transfer was successful
revert("Ether transfer should have reverted");
} catch Error(string memory /*reason*/) {
// Ether transfer should have reverted, continue testing
} catch {
// Unexpected error occurred, fail the test
revert("Unexpected error occurred");
}
​
// Perform the same checks for the secondProxy contract
try address(secondProxy).call{value: 1 ether}("") {
// Revert the test and fail if Ether transfer was successful
revert("Ether transfer should have reverted");
} catch Error(string memory /*reason*/) {
// Ether transfer should have reverted, continue testing
} catch {
// Unexpected error occurred, fail the test
revert("Unexpected error occurred");
}
​
// Additional test logic...
}
​

In these code snippets, the try statement is used to execute the Ether transfer and catch any exceptions that may occur. If the transfer was successful, the test case is reverted with an appropriate error message. If the transfer resulted in a revert due to the lack of a payable fallback function, the test case continues. This ensures that the test cases explicitly verify the behavior of Ether transfers to contracts without a payable fallback.

By adding these try-catch blocks, you can properly handle Ether transfers and prevent unintended transfers to contracts that are not designed to accept Ether.

Support

FAQs

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