Are there any security issues to send token in fallback function?

Question

I want to have a contract that releases an erc-20 token as soon as someone pays ether to contract. Let's call this contract Bid Contract. The abstract code goes like:

function askForTokens() public payable returns(bool){
    uint coins = msg.value / rate;
    token.transfer(msg.sender, coins);
    BidSuccessful(msg.sender, rate, msg.value, coins);
    return true;
}

function() payable{
    askForTokens();
}

So, now when someone sends ethers to contract from his wallet, the fallback functions gets executed and which in turn calls the askForToken function that will send corresponding tokens to msg.sender.

I have tested the functionality and this seems to be working fine.

What I am concerned about is is there any security threat in this implementation?

I can find everywhere suggestion of not writing the logic in fallback function, is the only reason is gas or this makes the contract more vulnerable?

Answer 1

1) This way askForTokens() will always run out of gas if your fallback function is called via the plain send() or transfer() functions. The send() and transfer() functions only forwards 2,300 gas, which is not enough to cover the gas needs of your askForTokens() function.

If your costumers don't use the address.call.value().gas()() or other non-intuitive solutions to forward ether (and more gas than the usual ether transfers), then they will never be able to buy tokens from you. More info here.

2) Why don't you just use the askForTokens() function? That way you don't have to bother about the limitations of the fallback function.

3) You have a mishandled exception error at the token transfer. You do not check whether the token transfer was successful or not. It might be the case that the transfer silently fails (it does not revert, it only returns a boolean; check the ERC20 transfer function out here) although your contract thinks that tokens have arrived and you emit an event.