search

ETHSydney / meetup-token

Ethereum token for meetup groups
MIT License
7 stars 3 forks source link

Reentry protection missing #1

Closed ltfschoen closed 7 years ago

ltfschoen commented 7 years ago

Existing functions transfer and transferFrom (for sending an amount of tokens from between addresses) and approval for approval of a third-party spender) currently contain the following comments Reentry protection prevents attacks upon the state suggesting that reentry protection is incorporated, for example Line 76 in transferableToken.sol.

However, there are no such reentry protection modifiers being called in public functions to prevent reentry attacks being made from external calls. We need to audit the code to cater for reentry attacks by adding anti-reentry logic.

Propose incorporation of a reentrant flag in methods and use of a mutex mechanism either implemented in a function within the existing contract (lock/unlock the contract to prevent reentry when its locked) or by using an external contract with message queue contract to perform the validation and guard against this (as described here, and use to compiler to prevent reentrancy actions (i.e. the contract being entered a second time whilst already in progress) by throwing a compiler error if a call is made from a non-reentrant method, or a call a non-reentrant method from a reentrant method so that functions without this flag would be prohibited from engaging in recursion.

Suggest using this ReentryProtected smart contract. Example usage: here and here

naddison36 commented 7 years ago

The meetup token was based on @o0ragman0o's ERC20 contract which had added reentry protection https://github.com/o0ragman0o/ERC20

Reentry protection is not needed for transferring ERC20 tokens. It's only needed when you are transferring Eth as that can invoke the default function on the receiving contract. I would remove the unnecessary comment but that will change the bytecode of the contract. Since the contract has already been deployed I'm going to leave it.

If I'm wrong on this then I'd love to see how this contract can be exploited. I'm happy to transfer some tokens to a contract you have deployed that has a reentry attack on this contract.