0

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?

| improve this question | |
0

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.

| improve this answer | |
  • I was also expecting the same as you said. The fallback function has very less gas available, so sending ether to contract will always fail. But this didn't happen in real case. But I said, I tested the code and this is working absolutely find. I can send ethers (via MEW or Metamask) and this returns me the tokens. – Prashant Prabhakar Singh Feb 21 '18 at 14:51
  • hmmm...strange! Did you send the ether from an externally owned account or a contract? I think from an externally owned account it should always fail while if you send it from a contract it should only work if you use the address.call.value().gas()() pattern I mentioned in my answer. Did you experiment about the 3rd point? What happens if your contract does not have enough tokens to cover the transfer for the msg.sender? It silently fails, although the event will be emitted...at least this is the behaviour I'm expecting – István András Seres Feb 21 '18 at 14:59
  • Yes. I did this from EOA. I will probably find a solution to this. ANd for 3rd point, I don't see a mess. Ideally, I should check return value while calling the transfer. But even I don't there won't be any issue. For eg, when the contract doesn't have sufficient tokens, and I try to call the transfer function of token, the token transfer function will throw and this will revert the changes. – Prashant Prabhakar Singh Feb 22 '18 at 4:48
  • The answer is not accurate. The 2,300 gas limitation only applies if another smart contract calls the ether transfer or send functions. If a user sends ether directly (from a wallet, for example), there is not gas limitation. That's why it's actually working. If you were sending ether through send or transfer from another contract, the fallback function would always fail. – Alex Cabrera Mar 25 '18 at 2:49
  • And be really carefull with the call function. The docs don't recommend using it, even with the .gas() modifier. Read a lot about how it works and behaves before using it to know if it's your best pick! – Alex Cabrera Mar 25 '18 at 2:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.