Ethereum smart contract security study and analysis

If you are new to Ethereum development, I recommend reading our Hitchhiker’s Guide to Ethereum Smart Contracts before reading this article.

Learning Ethereum smart contract security is a very hard job. There are only a few excellent guides and compilations, such as ConsenSys's Smart Contracts Best Practices, or Solidity Document Security Considerations. But if you don't write code yourself, it's hard to remember and internalize these concepts.

I’m going to try a slightly different approach. I’m going to explain some recommended strategies for improving the security of your smart contracts and show some code examples. I’m also going to show some code samples you can use to secure your smart contracts. Hopefully this will help create a kind of muscle memory of things, and serve as a mental reminder for writing actual code in the future.

Without further ado, let’s get into the best practices:

Code fails quickly, and talks about failure loudly

A simple and powerful good programming practice is to make your code fail as quickly as possible. And talk about it loudly. Let's look at an example of a function that behaves shyly:

 // UNSAFE CODE, DO NOT USE!

 contract BadFailEarly { uint constant DEFAULT_SALARY = 50000; mapping(string => uint) nameToSalary;

  function getSalary(string name) constant returns (uint) {
    if (bytes(name).length != 0 && nameToSalary[name] != 0) { return nameToSalary[name];
    } else { return DEFAULT_SALARY;
}
}
}

We want to quietly avoid a contract failing, or continuing in an unstable or inconsistent state. It’s great that the function getSalary is checking conditions before returning the stored salary. The problem is that when those conditions are not met, a default value is returned. This can hide errors from the caller. This is an extreme case, but this kind of programming is very common, and often it’s the fear of bugs that can break our apps. The fact is, the sooner we fail, the easier it is to find the problem. If we hide errors, they can propagate to other parts of the code, causing inconsistencies that are difficult to trace. A more correct approach is:

 contract GoodFailEarly { mapping(string => uint) nameToSalary; function getSalary(string name) constant returns (uint) {
    if (bytes(name).length == 0) throw;
        if (nameToSalary[name] == 0) throw; return nameToSalary[name];
}
}

This version also demonstrates another desirable programming pattern, separating preconditions so that each failure is isolated. Note that some of these checks (especially those that rely on internal state) can be implemented with function reconcilers.

Supports pull payment more than push payment

Every transfer of ether represents a potential code execution. The receiving address can implement a fallback function that may throw an error. Therefore, we should never trust that a send call will execute without error. Solution: Our contract should support pull payments over push payments. Take a look at the following innocent-looking bidding function code:

 // UNSAFE CODE, DO NOT USE!

 contract BadPushPayments { address highestBidder; uint highestBid; function bid() {
    if (msg.value < highestBid) throw;
if (highestBidder != 0) {
      // return bid to previous winner
      if (!highestBidder.send(highestBid)) {
throw;
}
}
    highestBidder = msg.sender;
highestBid = msg.value;
}
}

Notice that the contract calls the send function and checks its return value, which looks reasonable. But it calls send in the middle of a function, which is unsafe. Why? Remember, as mentioned above, send can trigger the execution of code in another contract.

Imagine someone bids from an address, and every time someone sends funds to that address, an error is thrown. What if someone tries to bid higher than this? The send call will always fail, making the bid abnormal. A function call that ends with an error leaves the state unchanged (any changes are rolled back). This means no one can bid, and the contract fails.

The simplest solution is to separate payments into a different function, letting users request (pull) funds independently from the rest of the contract logic:

 contract GoodPullPayments { address highestBidder; uint highestBid; mapping(address => uint) refunds; function bid() external {
    if (msg.value < highestBid) throw;

if (highestBidder != 0) {
      refunds[highestBidder] += highestBid;
}

    highestBidder = msg.sender;
highestBid = msg.value;
  } function withdrawBid() external {
    uint refund = refunds[msg.sender];
refunds[msg.sender] = 0;
    if (!msg.sender.send(refund)) {
      refunds[msg.sender] = refund;
}
}
}

This time, we use a mapping to store a refund value for each high bidder, and provide a function to withdraw their funds. In the event of a problem with the send call, only the bidder is affected. This is a simple pattern that solves many other problems (such as reentrancy). So remember: when sending ether, favor pull payments over push payments.

I have implemented a contract that you can use to easily use this pattern. Here is an example showing how to use it.

Organizing your function code: conditions, actions, interactions

As an extension of the fail-fast principle, a good practice is to arrange your functions in the following way: first, check all preconditions; then, change the state of your contract; and finally, interact with other contracts.

Condition, Action, Interaction. Sticking to this function structure will allow you to avoid a lot of problems. Let's look at an example of a function using this pattern:

 function auctionEnd() { // 1. Conditions
  if (now <= auctionStart + biddingTime)
    throw; // auction did not yet end
if (ended)
    throw; // this function has already been called

// 2. Effects
ended = true;
  AuctionEnded(highestBidder, highestBid); // 3. Interaction
  if (!beneficiary.send(highestBid))
throw;
}
}

This is in line with the fail-fast principle, as the condition is checked at the beginning. It also pushes potentially dangerous interactions with other contracts to the end.

Understand the limitations of the platform

The Ethereum Virtual Machine (EVM) has a lot of hard limits on what our contracts can do. These are platform-level security considerations, but if you don't understand them, they can threaten the security of your specific contract. Let's look at the following seemingly correct employee bonus management code:

 // UNSAFE CODE, DO NOT USE!

 contract BadArrayUse { address[] employees; function payBonus() {
    for (var i = 0; i < employees.length; i++) {
      address employee = employees[i];
      uint bonus = calculateBonus(employee);
employee.send(bonus);
}
       } function calculateBonus(address employee) returns (uint) {
    // some expensive computation ...
}
}

Reading the code: It is pretty straight forward and appears to be correct. However, it hides 3 potential issues based on platform limitations.

The first problem is that the type of i will be uint8, since that is the smallest type required to hold the value 0. If the array has more than 255 elements, the function loop will not terminate until the gas is exhausted. Better use explicitly typed unit to have no surprises and higher limits. Avoid declaring variables using var if possible. Let's fix this:

 // STILL UNSAFE CODE, DO NOT USE!

 contract BadArrayUse { address[] employees; function payBonus() {
    for (uint i = 0; i < employees.length; i++) {
      address employee = employees[i];
      uint bonus = calculateBonus(employee);
employee.send(bonus);
}
       } function calculateBonus(address employee) returns (uint) {
    // some expensive computation ...
}
}

The second thing you should consider is the gas limit. Gas is Ethereum's mechanism for charging for network resources. Every function call that modifies state consumes gas. Imagine that calculateBonus calculates the bonus for each employee based on some complex calculation, like calculating the profit of many projects. This will consume a lot of gas, which can easily reach the gas limit of the transaction or block. If a transaction reaches the gas limit, all changes will be reverted, but the fee is still paid. When using loops, be aware of variable gas costs. Let's optimize the contract by separating the bonus calculation from the for loop. Note that this is still problematic as the gas cost grows as the array of employees grows.

 // UNSAFE CODE, DO NOT USE!

 contract BadArrayUse { address[] employees; mapping(address => uint) bonuses;

function payBonus() {
    for (uint i = 0; i < employees.length; i++) {
      address employee = employees[i];
      uint bonus = bonuses[employee];
employee.send(bonus);
}
       } function calculateBonus(address employee) returns (uint) {
uint bonus = 0;
    // some expensive computation modifying the bonus... bonuses[employee] = bonus;
}
}

Finally, call stack depth limits. The EVM call stack has a hard limit of 1024. This means that if the number of nested calls reaches 1024, the contract will fail. An attacker could recursively call a contract 1023 times, then call a function in our contract, causing sends to silently fail due to this limit. PullPaymentCapable.sol was described above, and allows for easy implementation of pull payments. Inheriting from PullPaymentCapable and using asyncSend protects you from this.

Here is a modified version of the code that addresses all of these issues:

 import './PullPaymentCapable.sol';

 contract GoodArrayUse is PullPaymentCapable {
  address[] employees; mapping(address => uint) bonuses; function payBonus() {
    for (uint i = 0; i < employees.length; i++) {
      address employee = employees[i];
      uint bonus = bonuses[employee]; asyncSend(employee, bonus);
}
}

  function calculateBonus(address employee) returns (uint) {
uint bonus = 0;
    // some expensive computation...
    bonuses[employee] = bonus;
}
}

To summarize, it is important to remember (1) the limits on the types you use, (2) the limits on your contract’s gas costs, and (3) the limits on call stack depth.

Writing Tests

Writing tests is a lot of work, but it can save you from regression problems. Regression errors occur when a previously correct component becomes broken due to recent changes.

I'll be writing a more extensive guide on testing soon, but if you're curious you can check out Truffle's testing guide.

Fault tolerance and automated bug bounties

Thanks to Peter Borah for inspiring both of these ideas. Code reviews and security audits are not enough to be safe. Our code needs to prepare for the worst-case scenario. There is a vulnerability in our smart contract, there should be a way for it to recover safely. Not only that, but we should try to catch these vulnerabilities as early as possible. And incorporating automated bug bounties into our contracts can help.

Let’s take a look at this simple automated bug bounty implementation in a hypothetical token contract:

 import './PullPaymentCapable.sol';import './Token.sol';

 contract Bounty is PullPaymentCapable { bool public claimed; mapping(address => address) public researchers;

function() {
if (claimed) throw;
  } function createTarget() returns(Token) {
    Token target = new Token(0);
    researchers[target] = msg.sender; return target;
  } function claim(Token target) {
    address researcher = researchers[target];
    if (researcher == 0) throw;

    // check Token contract invariants
    if (target.totalSupply() == target.balance) { throw;
}
    asyncSend(researcher, this.balance);
claimed = true;
}
}

As before, we use PullPaymentCapable to secure our payments. This Bounty contract allows researchers to create a copy of the Token contract we want to audit. Anyone can contribute to the bug bounty by sending a transaction to the Bounty contract address. If any researcher manages to corrupt his copy of the Token contract (for example, in this case, making the total supply of tokens different from the Token balance), he will get the bounty reward. Once the bounty is claimed, the contract will not accept more funds (the unnamed function is called the contract's rollback function, and it is executed every time the contract is directly sent funds).

As you can see, this has the nice property that it is a separate contract and does not require modifications to our original Token contract. Here is a full implementation available to anyone on GitHub.

As for fault tolerance, we will need to modify our original contract to add additional safety mechanisms. A simple idea is to have a contract manager freeze the contract as an emergency mechanism. Let's see a way to implement this behavior through inheritance:

 contract Stoppable { address public curator; bool public stopped;

  modifier stopInEmergency { if (!stopped) _ } modifier onlyInEmergency { if (stopped) _ } function Stoppable(address _curator) {
if (_curator == 0) throw;
curator = _curator;
  } function emergencyStop() external {
    if (msg.sender != curator) throw;
stopped = true;
}
}

Stoppable allows to specify an address of a manager who can stop the contract. What does "stopping a contract" mean? This is defined by the child contract inherited from Stoppabl by using the function modifiers stopInEmergency and onlyInEmergency.

Let’s look at an example:

 import './PullPaymentCapable.sol';import './Stoppable.sol';

 contract StoppableBid is Stoppable, PullPaymentCapable { address public highestBidder; uint public highestBid; function StoppableBid(address _curator)
Stoppable(_curator)
    PullPaymentCapable() {} function bid() external stopInEmergency {
    if (msg.value <= highestBid) throw;

if (highestBidder != 0) {
      asyncSend(highestBidder, highestBid);
}
    highestBidder = msg.sender;
highestBid = msg.value;
  } function withdraw() onlyInEmergency {
suicide(curator);
}
}

In this example, bidding can now be stopped by a curator, which is defined when the contract is created. StoppableBid is in normal mode, with only the bid function being called. If something weird happens and the contract is in an inconsistent state, the curator can step in and activate an emergency state. This makes the bid function unable to be called, allowing the withdraw function to run.

In this case, the emergency mode will just let the administrator destroy the contract and recover the funds. But in a real case, the recovery logic may be more complex (such as returning funds to their owners). Here is an implementation available to anyone on GitHub.

Limit the amount of funds deposited

Another way to protect our smart contracts from attacks is to limit their scope. Contracts that manage millions of dollars are most likely to be targeted by attackers. Not all smart contracts need to be so risky. Especially if we are experimenting. In this case, it may be useful to limit the amount of funds our contract accepts. This is as simple as adding a hard limit to the contract address balance.

Here's a simple example of how to do this:

 contract LimitFunds { uint LIMIT = 5000; function() { throw; } function deposit() {
    if (this.balance > LIMIT) throw;
...
}
}

This short fallback function will reject any direct payments to the contract. If the contract's balance exceeds the desired limit or something goes wrong, the deposit function will be checked first. More interesting things like dynamic or managed limits are easy to implement.

Writing simple module code

Safety comes from a match between our intentions and what our code is actually allowed to do. This is very difficult to verify, especially if the code is huge and messy. This is why it is important to write simple modular code.

This means that functions should be as short as possible, code snippets should be reduced to a minimum, files should be as small as possible, and independent logic should be split into modules, each of which has some single responsibility.

Naming is also one of the best ways to express our intentions while coding. Think carefully about the names you choose to make your code as clear as possible.

Let's look at an example of bad naming. Let's look at a function from The DAO. I'm not going to copy the function code here because it's quite long.

The biggest problem is that it is too long and complex. Try to make your functions shorter, 30 to 40 lines of code at most. Ideally, you should be able to read the functions and understand what they do in less than a minute. Another problem is the poor naming of the Transfer event on line 685. This name is only one character different from a function called transfer. This will confuse everyone. In general, the recommended naming of events is that they should start with 'Log'. In this case, the name LogTransfer would be a better choice.

Remember to keep your smart contracts as simple, modular, and well-named as possible. This will greatly facilitate auditing your code by others and yourself.

Don't write all your code from scratch

Finally, as the old saying goes: "Don't move your crypto". I think it applies to smart contract code as well. You are dealing with money, your code and data are public, and you are running on a new experimental platform, the stakes are high. There is the potential for chaos everywhere.

These practices help to protect our smart contracts. But ultimately, we should create better development tools to build smart contracts. There are some interesting initiatives here, including better type systems, Serenity Abstractions, and the Rootstock platform.

There is a lot of well-written and secure code out there, and frameworks are starting to emerge. We've started compiling some of these best practices in this GitHub repo we call OpenZeppelin. Feel free to take a look and contribute new code or security audits.

Summarize

In summary, the security model described in this article:

1. Code fails quickly, and talks about failure loudly

2. Supports pull payment more than push payment

3. Organizing your function code: conditions, actions, interactions

4. Understand the limitations of the platform

5. Writing Tests

6. Fault tolerance and automated bug bounties

7. Limit the amount of funds deposited

8. Writing simple module code

9. Don't write all your code from scratch

If you want to join the discussion about secure smart contract development patterns, join us on slack. Let’s improve the state of smart contract development together!

To stay up to date on our smart contract security work, follow us on Medium and Twitter.