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

Medium - MiniMeToken.sol - checks not done properly #135

Open
k4ms opened this issue Jun 18, 2017 · 2 comments
Open

Medium - MiniMeToken.sol - checks not done properly #135

k4ms opened this issue Jun 18, 2017 · 2 comments

Comments

@k4ms
Copy link

k4ms commented Jun 18, 2017

As stated in the Solidity docs (http://solidity.readthedocs.io/en/develop/security-considerations.html#use-the-checks-effects-interactions-pattern), it is much safer to use the Checks-Effects-Interactions Pattern.

We can see 3 occurrences in the source code of MiniMeToken.sol where this pattern should be used to avoid improper situations :

  • doTransfer (l.239) : the check overflow is done too late and if the condition is met, amount can be deduced from sender and not credited to receiver : the account balance is not respected.

  • generateTokens (l.425) : totalSupplyHistory can be updated without balances to be : possible owner account balance value error.

  • destroyTokens (l.442) : again totalHistory can be decremented without owner balance to be.

Each checks/throw should be done before doing any update of variable.

@jbaylina
Copy link
Contributor

In solidity, when a throw is raised, it rolls back the transaction, so it's not possible to trick the balances the way it's described here.

@k4ms
Copy link
Author

k4ms commented Jun 18, 2017

Yes you are right for transactions but my point was more for the internal contract variables (totalSupplyHistory, balances) that can be corrupted. In case of a throw, do these variables stay in their previous state?
I was concerned because these variables are used for other computations and contracts.

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