Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor issue: It is possible to transfer more than user balance when transfering to self with transferFrom #375

Closed
yaronvel opened this issue Aug 16, 2017 · 3 comments

Comments

@yaronvel
Copy link
Contributor

In transferFrom first receiver balance is increment and only then spender balance is decremented.
This allows to send more than you have. It is very minor as you send it to your self and total balance is not affected.
However, it is inconsistent with transfer where you first decrements sender balance and only then increment receiver balance.

@frangio
Copy link
Contributor

frangio commented Aug 17, 2017

Great find, @yaronvel!

Just to clarify for everyone who sees this, this is only a minor problem because the sender's balance remains the same. However, attempting a transfer of more than the balance should fail.

I think this is actually an oversight in the ERC20 spec, too. For transfer it specifies that it should throw if the sender doesn't have enough balance, but it doesn't say anything for transferFrom.

Do you want to submit a PR swapping the sub and add lines, @yaronvel?

@yaronvel
Copy link
Contributor Author

Thanks, I will submit PR soon.
One should also be aware that Transfer event will be logged. So if some services rely on this log (e.g., measure daily volume), it could be a real issue.

@frangio
Copy link
Contributor

frangio commented Aug 19, 2017

That's correct. Thanks for reporting and fixing! (Fixed in #377)

@frangio frangio closed this as completed Aug 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants