Just saw a post on about a great idea for an smart contract called “ ” and decided to take a look at how it works. This post describes some of the bugs we found. Hacker News Ethereum Go Freaking Do It Intro My friend and are both interested in information/cyber security, and have recently taken notice to the attack surfaces available in Smart Contracts. Currently, we’re looking mainly at Solidity as well as EVM bytecode, trying to automate the process of finding vulnerabilities as well as doing some manual code audits. Nico I The code for the contract is and is available for auditing directly there. If you’re unfamiliar with Solidity, take a . Props to who is the author of the contract, and executed such a neat idea. on Etherscan look at this cheat sheet Karolis Ramanauskas What is “Go Freaking Do It” The concept is a simple: you deposit $$$ into the contract together with the goal you’d like to accomplish, as well as an optional “supervisor” contact who will verify if you’ve accomplished the goal or not by a given date. If they say you’ve done it, then you get your $$$ back. If not, “Go Freaking Do It” keeps it and the owner earns some money for their effort of building this. Low Severity Problems Found Not checking if the goals already contain given hash. Could result in lost goals (and $$$) if a goal with identical sender/description/value/deadline are submitted. Data Loss. function setGoal() {(...snip for brevity...) bytes32 hash = keccak256(msg.sender, \_description, msg.value, \_deadline); Goal memory goal = Goal(...snip for brevity...); goals\[hash\] = goal; (...snip for brevity...) } Solution: Either reject duplicates or add a nonce to the hash. This contract has two classes of users, “owner” and everyone else. The problem with this is that the contract has a few house keeping functions which are presumably run on an external system somewhere (such as setEmailSent()), and this system will then have access to credentials which can do absolutely anything to the contract. (syphon the money, transfer ownership, destroying it.). One super user for all tasks. Solution: It would be better to tier the permissions and have an additional account, “accountant” or similar. Because all goals are public, then you can see the e-mail addresses, goals, and amounts entered by all users. Data Leak. Goal[] public activeGoals; Solution: Don’t mark as public. High Severity Problems Found The “onlyOwner” guard is missing for setGoalFailed. This means that anyone can “fail” the goals of every user in the contract at any time, essentially donating all their money to the contract owner. No permissioning for setting a goal as failed. function setGoalFailed(uint _index, bytes32 _hash) {assert(goals[_hash].amount > 0);// assert(goals[_hash].emailSent == true); goals\[\_hash\].completed = false; activeGoals\[\_index\].completed = false; owner.transfer(goals\[\_hash\].amount); // send ether to contract owner setGoalFailedEvent(\_hash, false); } Solution: Add the onlyOwner guard to the function declaration. Because all of the state is stored in the contract itself, then upgrading is impossible. No Versioning or upgrade possibilities. Solution: Having separate business and data logic, alternatively, making use of delegatecall could help here. See here and here for great articles about upgradable contracts. Who we are Two FinTech CTOs for hire, based in London, UK. More about | More about . Jesper Nico Notes Shortly before publishing this post, user “mpeg” on HN . discovered the same setGoalFailed() bug Update Awesome news! 😎 The author pointed out that he’s working on a V2 and will address the outstanding issues.
