SafeERC20 safeTransfer,safeTransferFrom,.. gives no indication of failure when called on a contract which does not have transfer function and has a non reverting fallback function

[guylando]

Call

SafeERC20.safeTransfer(token, to, balance);

For the following contract passed as token:

contract NoTransferMock {
    address private owner;
    constructor () public
    {
        owner = msg.sender;
    }

    function () external payable {
        // solhint-disable-previous-line no-empty-blocks
    }

    function balanceOf(address) public pure returns (uint256) {
        return 10;
    }
}

then safeTransfer will call callOptionalReturn which which perform a .call(..) for the non existent transfer function and it calls the fallback function and returns for this contract success=true and empty returndata which pass successfully the require statement and if and end the callOptionalReturn execution without revert and without any indication that the transfer did not happen.

If a solidity function has IERC20 token parameter then in runtime it allows to pass the contract above and cause the behavior described.

I think this is a problem that anybody who uses safeTransfer thinks that either transfer happened or revert happens but I showed a third case which might be used by an attacker for code flows manipulation.

[guylando]

More than that, this same "attack" will work with a more standard token which has a non reverting fallback function and implements allowance and transferFrom and does not implement "transfer" (I imagine such scenario could happen because many security resources recommend to use approve+transferFrom instead of transfer so it totally can happen that someone will decide not to make transfer available at all just as Golem token did not implement transferFrom https://etherscan.io/address/0xa74476443119A942dE498590Fe1f2454d7D4aC0d#code so same attack would work on Golem token if it had a fallback function and SafeERC20.safeTransferFrom was called)

[guylando]

closing because seems this is the intended solidity behavior. callers should be careful and audit the contract they call to avoid such problems