The function FestivalPass::withdraw lacks security validation for the target parameter.
Description
-
if the target
is a contract, rather than an EOA (Externally Owned Account) address.
-
If the target contract does not have the appropriate functions to handle incoming Ether, the ETH will remain stuck in the target contract permanently.
function withdraw(address target) external onlyOwner {
@> payable(target).transfer(address(this).balance);
}
Risk
Likelihood:
Impact:
Proof of Concept
the administrator calls the withdraw
function with the address of the BlackHoleReceiver
contract as the parameter.
pragma solidity 0.8.25;
contract BlackHoleReceiver {
receive() external payable {
}
function getBalance() external view returns (uint256) {
return address(this).balance;
}
}
Recommended Mitigation
Ensure the withdraw
function is designed to fully support secure fund transfers.
// To maximize the likelihood of a successful transfer;
function withdraw(address target) external onlyOwner {
+ require(target != address(0), "Invalid target");
- payable(target).transfer(address(this).balance);
+ (bool success, ) = payable(target).call{value: address(this).balance}("");
+ require(success, "ETH transfer failed");
}
Ensure the target contract has properly implemented complete and secure functions for handling ETH transfers.
contract SafeReceiver {
address public owner;
constructor() {
owner = msg.sender;
}
receive() external payable {}
function getBalance() external view returns (uint256) {
return address(this).balance;
}
function withdraw(address payable to, uint256 amount) external {
require(msg.sender == owner, "Not owner");
require(to != address(0), "Invalid target");
require(address(this).balance >= amount, "Insufficient balance");
(bool success, ) = to.call{value: amount}("");
require(success, "ETH transfer failed");
}
}