Lines of code

Vulnerability details

Impact

transfer function can cause withdrawal to fail

Proof of Concept

function withdraw(
    address _assetAddress,
    address _to,
    uint256 _amount
) public {
    LibDiamond.enforceIsContractOwner();
    address sendTo = (_to == address(0)) ? msg.sender : _to;
    uint256 assetBalance;
    if (_assetAddress == NATIVE_ASSET) {
        address self = address(this); // workaround for a possible solidity bug
        assert(_amount <= self.balance);
        payable(sendTo).transfer(_amount);   @audit   can revert , change for send
    } else {
        assetBalance = IERC20(_assetAddress).balanceOf(address(this));
        assert(_amount <= assetBalance);
        IERC20(_assetAddress).safeTransfer(sendTo, _amount);
    }
    emit LogWithdraw(sendTo, _assetAddress, _amount);
}

The original function transfer is limiting the gas used to 2300 by design.
This is ok if sendTo is a wallet. However, if it is a smart contract it will fail in some cases

1. The smart contract does not implement a payable function.
2. The smart contract does implement a payable fallback which uses more than 2300 gas.
3. The withdrawer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call’s gas usage above 2300.

The original idea of this function was to avoid reentrancy.

Recommended Mitigation Steps

Use call instead

The text was updated successfully, but these errors were encountered:

All reactions