Common attacks in Solidity and how to defend against them

Published in

Coinmonks

14 min readSep 7, 2018

“black and gray laptop computer turned on” by Markus Spiske on Unsplash

Your heart sinks. Your breathing slows so much, you appear to be holding your breath. Your eyes widen and you feel sick to your stomach as you come to terms with the number glowing on the screen in front of you: your smart contract has been hacked. Somehow.

Let’s avoid this situation — at all costs — shall we? Let’s make ourselves aware of all known attack types and teach ourselves how to avoid them so we can sleep soundly at night.

Resources

Before we dive in, I highly, highly recommend supplementing what you learn in this article by going through OpenZeppelin’s Etherenaut, a “game” where you open up the browser console and use the injected web3 object to make RPCs (remote procedure calls) to the Ropsten test network to hack vulnerable contracts in order to progress in the game. It even makes you go to the Remix IDE to write and deploy malicious contracts to Ropsten so that you can call your custom functions in those contracts to exploit vulnerabilities in the Ethernaut contracts.

This product is brilliant and I consider it mandatory practice for any aspiring or current smart contract developers.

Race conditions: reentrancy

mapping (address => uint) private userBalances;

function withdrawBalance() public {
    uint amountToWithdraw = userBalances[msg.sender];
    msg.sender.transfer(amountToWithdraw);
    userBalances[msg.sender] = 0;
}

In the above function, notice how userBalances[msg.sender] is set to 0 after the ether is sent to the user. If this function is called rapidly multiple times (such as with a script, or by creating and deploying your own malicious contract that calls this function then recursively calls itself), it is possible to send userBalances[msg.sender] multiple times before it is set to 0. This type of exploit is how the DAO was hacked in 2016 (resulting in the community forking of ETH and ETC).

The best way to avoid this is to simply move the msg.sender.transfer(amountToWithdraw) line below the userBalances[msg.sender] = 0 line so that funds are transferred after the balance is zeroed. “What if the transfer fails?” you ask. Well, the function will be reverted so that userBalances[msg.sender] does not wrongfully get set to 0.

Race conditions: cross-function state dependency

mapping (address => uint) private userBalances;

function transfer(address _recipient, uint _amount) {
    require(userBalances[msg.sender] >= _amount);
    userBalances[_recipient] += _amount;
    userBalances[msg.sender] -= _amount;
}

function withdrawBalance() public {
    uint amountToWithdraw = userBalances[msg.sender];
    msg.sender.transfer(amountToWithdraw);
    userBalances[msg.sender] = 0;
}

Similar to the first race condition vulnerability, if you have two functions that rely on the same contract state, it is possible to call one function halfway through the other function’s execution and get undesirable results. Above, it is possible to call both functions simultaneously causing userBalances[recipient] += amount (first function) to fire off, then have msg.sender.transfer(amountToWithdraw) fire off in the second function, then have userBalances[msg.sender] -= amount fire off in the first function. This will result in the user being able to send money to another address (which they probably own too), then withdraw that same amount of money for themselves (stealing it from the contract balance).

Again, the way to defend against this is the same as in the first race condition: always make your state changes before transferring ether.

Integer overflows/underflows

Did you see something else that was vulnerable in the above example? If the hacker successfully transfers all their money to a secondary account, then userBalances[msg.sender] -= amount fires off when userBalances[msg.sender] probably has not a whole lot of money left in it, then userBalances[msg.sender] will underflow.

Overflow/underflow happens when uint s are incremented/decremented past their limit. The max value for a uint (which is an alias for uint256) is 2 ^ 256 – 1. If an integer increments past that number, it will overflow, and the value will go back to 0. This is true for all uint sizes, such as uint8 which, for example, is significantly smaller (max size = 2 ^ 8 - 1 = 255). An example of an overflow:

uint8 myNumber = 255;
    
function increment(uint _number) public returns (uint) {
    _number += _number;
    return _number;
}
    
uint newNumber = increment(myNumber); // newNumber is now 0, not 256

This can happen with underflows too. Any uint that is reduced below zero will jump to it’s max value. Can you imagine the shock of seeing some contract’s user’s uint balance = 0 jump to 1.157920892E77 — 1?!?

To prevent this, use the SafeMath library (built by the incredible folks at OpenZeppelin), which reverts any function that is called that would prevent an overflow. Can’t wait until this functionality comes standard in Solidity!

Contract balance dependencies

Did you know that when deploying a contract for the first time, it is entirely possible to precompute that contract’s address and send ether to the address before the contract is deployed?? Never assume your contract is being deployed with a balance of 0 ether.

You’re also able to forcibly send ether to any contract, affecting it’s total balance. Note that even if you have a fallback function that is not payable:

contract exampleContract {
    function() { } // The fallback function
}

…you’re still able to send ether to the contract! How?!? You can’t do it with a wallet address (i.e. your personal ETH account), but you can do it with a contract address like so:

contract etherBomb {
    address recipient;    // First, send this contract some ether
    // Then, call the below function to set a recipient address    function setTarget(address _target) public {
        recipient = _target;
    }    // Then call this function to destroy this contract
    // and forcibly send the funds anywhere    function kill() public { 
        selfdestruct(recipient);
    }
}

Because of all these things, it’s always a good rule to be extremely mindful whenever dealing with contract balance in your app logic, especially in if or require statements.

Denial of service

I mentioned this in the previous vulnerability, but it’s worth repeating: not all addresses that interact with your contract will be wallet addresses — other contracts can call your code too. Because of this, they can be used to break your app. Consider the following:

contract Auction {
    address highestBidder;
    uint highestBid;    function bid() public {
        require(msg.value < highestBid);         if (highestBidder != 0x0) { 
            highestBidder.transfer(highestBid); // Return old bid
        }        // Store new highest bid and bidder
        highestBid = msg.value; 
        highestBidder = msg.sender;
    }
}

If there was a malicious contract that looks like this:

contract AuctionAPI {
    function bid(); // Exposes function after contract instantiation
}contract Attacker {
    address auctionAddress = 0x0123456789abcdef; // Put address here
    AuctionAPI auctionContract = AuctionAPI(auctionAddress);    function placeBid(uint _amount) public {
        auctionContract.bid.value(_amount); 
    }    function () payable {
        throw; // Rejects the payment before the function completes
    }
}

Assuming it has some ether in it, the Attacker contract could send its ether to the Auction contract by calling the bid() function and become the highestBidder. Because the Auction contract returns the old highest bid to its owner when a new highest bid comes in, it will try to send the money back to Attacker when a new highest bid comes in. But since Attacker has a fallback function that throws, it rejects the payment and sends it back to Auction, and Auction stops the bid function before it can set a new highest bid/bidder. Effectively, this paralyzes the contract.

A good way to defend against this is to implement a “balance withdrawal” design pattern (more on that below) so that any ether transfer functionality is independent from other app logic.

Timestamping / miner tampering

Miners are capable of ordering transactions within a block. This is a well-known problem in the dEX space called “frontrunning”, where certain buy/sell orders in the same block can be rearranged to execute in a different sequence than expected. Additionally, miners are also capable of adjusting the block timestamps (within the range of about 30 seconds), so it is never good practice to use block.timestamp — use block.number instead. If you must use a timestamp and its accuracy is critical to your app, consider pulling that info from an Oracle with authenticity proofs, or consider the Ethereum Alarm Clock. To learn more about why timestamping in Ethereum is a problem, check out (and upvote) this wonderful StackExchange answer.

Alternatively if the situation calls for it, you could implement a “commit/reveal” design pattern (more on that below) if it’s problematic to have the world be able to see the data of an about-to-be-mined transaction.

Delegatecall

According to the Solidity docs:

There exists a special variant of a message call, named delegatecall which is identical to a message call apart from the fact that the code at the target address is executed in the context of the calling contract and msg.sender and msg.value do not change their values.
This means that a contract can dynamically load code from a different address at runtime. Storage, current address and balance still refer to the calling contract, only the code is taken from the called address.
This makes it possible to implement the “library” feature in Solidity: Reusable library code that can be applied to a contract’s storage, e.g. in order to implement a complex data structure.

Improper security around the use of this function was the cause for the Parity Multisig Wallet hack in 2017. TL;DR this function is very useful for granting your contract the ability to make calls on other contracts, as if that code were a part of your own contract. However, using delegatecall() causes all public functions from the called contract to be callable by anyone — usually not a problem when using public libraries. But because this behavior was not recognized when Parity built its own custom libraries…

Improper security measures were placed in the called library.
This allowed the hacker to reinitialize the wallet by placing his/her own addresses as the multisig authorizers.
Now in possession of all multisig authorizer addresses, the hacker was able to pull out all the contract’s funds.

To dive deeper into this scenario, check out this awesome write-up.

The takeaway: be very careful when relying on delegatecall() when you organize your code into multiple contracts, libraries, and packages — you must have total end-to-end security measures in place to prevent unwanted storage modifications.

“bald eagle door chain lock” by MILKOVÍ on Unsplash

General contract-writing best practices

If you stick to writing solid, clean code, there’s less risk of vulnerabilities being introduced. These good habits should become second nature to you.

Function / data structure exposure

It is best practice to explicitly state the permission level of each of your functions and data structures so that you are completely aware of what functions and data are visible to or callable by wallet addresses or other contract addresses. If you’re unfamiliar with the differences between external, internal, public, and private, check out the Solidity documentation.

Contract ownership

It is wise to use OpenZeppelin’s Ownable package to establish a single address who has administrative privileges and can call owner-specific functions. This can be incredibly useful, especially with the “circuit breaker” functionality below.

Circuit breaker

bool public contractPaused = false;function circuitBreaker() public onlyOwner { // onlyOwner can call
    if (contractPaused == false) { contractPaused = true; }
    else { contractPaused = false; }
}// If the contract is paused, stop the modified function
// Attach this modifier to all public functions
modifier checkIfPaused() {
    require(contractPaused == false);
    _;
}

It is wise to implement a “circuit breaker” function modifier that prevents all app functionality if a catastrophic event occurs. This functionality should be used in tandem with Ownable so that only the contract owner can pause/unpause the app. This is useful for buying time in dire situations and analyzing what went wrong and how to fix it. Though some users may protest about the lack of decentralization in this approach, sometime after deployment you could simply transfer contract ownership to the 0x0 address, preventing anyone from calling Ownable functions.

Loops

struct Account {
    address userAddress;
    uint balance;
}Account[] private accounts;function withdrawAllBalances() public {
    for (uint i = 0; i < accounts.length; i++) {
        Account storage user = accounts[i];
        user.userAddress.transfer(user.balance);
        user.balance = 0;
    }
}

Be very careful when writing loops, as they can quickly reach the block gas limit, especially with loops over data structures of dynamic size (such as an array of addresses, where you can add more addresses to that array).

Sanitize incoming data / function arguments

Your DApp’s front end isn’t the only place that could send your contract RPCs, so you must sanitize any incoming data or function arguments before inserting them into storage. Never trust anything outside your contract.

Fail early, fail loud

function usesIfStatements() {
    if (msg.sender == owner) { // Will fail silently
        doSomething();
    }
}function usesRequire() {
    require(msg.sender == owner); // Will fail loudly
    doSomething();
}

Always favor require() statements over if statements wherever possible, because if the require() statement is not met, it will throw a revert that is transmitted to the function caller (such as a front end) and stop the function right then and there. if statements tend to fail silently, and can also lead to deeply-nested code that is harder to read.

Place such statements at the top of your functions whenever possible to check for these conditions as early as possible, to save on gas costs and to ensure no unwanted functionality is executed.

“empty gray concrete pathway with roof” by Alex J. Reyes on Unsplash

Useful design patterns

A number of the above vulnerabilities can be mitigated by implementing tried-and-true design patterns in your code. Here’s a few useful ones.

Balance withdrawal (aka “pull” vs “push” payments)

mapping (address => uint) private balances;function depositFunds() public {
    balances[msg.sender] += msg.value;
}function checkBalance() public returns (uint) {
    return balances[msg.sender];
}function withdrawFunds(uint _amount) public {
    require(balances[msg.sender] >= _amount);
    balances[msg.sender] -= _amount; // Reverts if transfer fails    // This is what we want separated from other app logic
    msg.sender.transfer(_amount); 
}function sendFunds(address _receiver, uint _amount) public {
    require(balances[msg.sender] >= _amount);
    balances[msg.sender] -= _amount;
    balances[_receiver] += _amount;
}

This design pattern separates the action of sending ether from all other app functionality. You can keep track of the balances of all users, allow them to deposit/withdraw funds in/out of the contract, allow them to do things in the contract with those funds (without actually sending ether out of the contract), and only when ether is actually being sent out of the contract should you use .transfer(), and it should be in its own function just in case the transfer fails.

State-reliant actions (aka “state machine”)

struct Order internal {
    string from;
    string to;
    uint price;
    uint Status;
}mapping (uint => Order) public orders; // orderIdenum Status {
    Open,
    Cancelled,
    AwaitingShipping,
    InTransit,
    Delivered,
    Complete
}function markAsDelivered(uint _orderId) public {
    require(orders[_orderId].Status == Status.InTransit);
    orders[_orderId].Status = Status.Delivered;
}

One incredibly useful design pattern is the “state machine”, a way to limit certain actions from being taken unless a certain state condition is met. In the above shipping example, an order cannot be marked as delivered unless its current status is InTransit. Setting up these conditions ensures a logical, enforceable progression of steps. This design pattern allows for an incredible amount of intricacy to be feasible in your app — this is by far my favorite!

Commit/reveal

The commit/reveal design pattern is best used when a user’s actions or choices might influence another user’s actions or choices, such as a vote, or a move in a game where all players’ moves are revealed at the same time, or even a blind auction. Basically, you structure the DApp into two separate phases (with a “state machine”!): a commit phase, and a reveal phase.

In the commit phase, users submit an encrypted hash of their move/vote/etc, which is stored in the contract. No one knows what the actual move/vote/etc is by looking at contract storage, as they only see a hash. Update: Thanks Roman Storm for pointing out that you should use web3.utils.soliditySha3() on the front end to encrypt values before transmitting them to the blockchain (as opposed to receiving raw, unencrypted, exposed data into the contract, which the contract then encrypts), since the data in those transactions is public!
After all users commit, we enter the reveal phase. Users must re-submit their original move/vote/etc to prove that, by hashing it, it matches what they originally committed. This validates the action.

To learn more about this design pattern, check out this fantastic article. If you want to see an example of this code, check out my decentralized rock-paper-scissors game.

Speed bumps / action throttling

mapping (address => uint) private userLastAction;
uint throttleTime = 30 seconds; // Attach this to critical functions, such as balance withdrawals
modifier speedBump() {      
    if (!userLastAction[msg.sender]) {
        userLastAction[msg.sender] = 0;
    }    require(now - throttleTime >= userLastAction[msg.sender]);
    userLastAction[msg.sender] = now; // now == block.timestamp
    _;
}

This design pattern can be used to throttle user actions. Yes, this eats up a lot of gas because of storage costs. But in apps where only a few, extremely critical actions are taken, this design pattern can be beneficial. Above, I used now a.k.a. block.timestamp for illustrative purposes, though it would be better to use block.number instead and have uint throttleTime = 1; // 1 block.

Automatic deprecation

uint public expirationBlock = 7654321;// Attach this modifier to all public functions
modifier preventIfDeprecated() {
    require(expirationBlock >= block.number);
    _;
}

This pattern is useful for shutting down a contract at a specified time (such as with an alpha/beta test period) without having to use selfdestruct() and subsequently losing all your contract’s storage data.

Data segregation

If you’re looking to build upgradable contracts, you should keep all app data in a separate contract and build in conditions for what addresses (your most recent version of your DApp) are allowed to modify that contract’s state. This is relatively advanced and I won’t go into it in depth here, but if you want to know more check out this basic code snippet and this high-level article and this in-the-weeds article. If you really want to dive into the utmost detail, check out this Ethereum R&D article about it.

Credits

I learned a great deal of this information from the Consensys Academy Developer Program. Some of the code snippets in this article were directly inspired (though not outright plagiarized) by the examples they showed us in the course.

If you catch anything in this article that needs correction, please drop me a comment and let me know! And if your project is looking for a part-time or contract smart contract developer, reach out to me! I offer development and auditing services, as well as product management services for the right project.

I manage product/strategy/bizdev at OMGpool.org, where we’re building the first open-source, community-driven staking pool for OmiseGO. I also write smart contracts — check me out at github.com/joelsfoster.

Be sure to check out our staking rewards calculator and news feed on our website, and stay tuned for new features we’re adding while we wait for staking to arrive.

If you’d like to be the first to hear about the staking pool when it launches, sign up for email updates below:

Follow us on Twitter | Medium | Telegram |Reddit

Email us at: contact@omgpool.org

Join Coinmonks Telegram Channel and Youtube Channel get daily Crypto News