sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

Udsen - The transferred funds could be lost if zero address is passed in as function argument #408

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Udsen

medium

The transferred funds could be lost if zero address is passed in as function argument

Summary

The function transferETH(address payable to, uint256 amount) in the UXDTimelockController.sol is not conducting the check for the zero address for the passed in address parameter to address.

Vulnerability Detail

Since this function is only callable by the onlySelf which means by the contract itself (governance), and if by mistake a zero address is passed in as the to address the transfered ETH will be lost forever.

Impact

value: amount the amount which is passed in through the low level call function will be lost.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/governance/UXDTimelockController.sol?plain=1#L40-L44

function transferETH(address payable to, uint256 amount) external onlySelf nonReentrant {
    // solhint-disable-next-line avoid-low-level-calls
    (bool success,) = to.call{value: amount}(""); //@audit - needs to perform the require(to != address(0), "Zero Adddress")
    require(success, "Failed to send ETH");
}

Tool used

Manual Review, VSCode

Recommendation

conduct the check for the zero address for the to address passed in. require(to != address(0), "Zero Adddress")

So the updated code snippet will be as follows:

function transferETH(address payable to, uint256 amount) external onlySelf nonReentrant {
    require(to != address(0), "Zero Adddress");
    // solhint-disable-next-line avoid-low-level-calls
    (bool success,) = to.call{value: amount}(""); //@audit - needs to perform the require(to != address(0), "Zero Adddress")
    require(success, "Failed to send ETH");
}