A beginner's guide to Ethereum development: Part 2

A beginner's guide to Ethereum development: Part 2
Photo by Richard Horvath

This post is a continuation of the first post, if you haven't given that a read yet, go for it! I'll try to delve a bit more in-depth in this particular post so best to keep some resources handy.

Reducing the attack surface

This is one of the most common phrases you'll hear in smart contract audits, and for good reason - and it's the same reason you wear your seat belt when driving or carry a parachute when skydiving. There's an interesting principle that applies to literally everything that happens, Murphy's law, it's not a law in the strict sense of the word's meaning but the law has probably been proven so many times in the entire life of every single human being that "prevention is better than cure" and "a stitch in time saves nine" are childhood sayings for most of us. The same applies here.

Anyway, on that note, let's take a trip through history.

Story time

There was a DAO, called The DAO (think of it as the OG DAO), that came into being on 30th April 2016. Hot off the press after a 28-day crowdsale, the entire ecosystem couldn't wait to see what came of this new autonomous and mostly decentralized being, it was being pitched as the next step in Web3, heck, even the word Web3 wasn't popular then probably. To give you an idea of how big this was, according to The Economist, the DAO had nearly 14% of the entire ether supply then, about $150 million of value at that time.

Soon after, in May, there was a paper released that talked about some vulnerabilities in the contract. In June, an Ethereum developer on GitHub pointed out recursive call vulnerabilities, which subsequently got blogged by the then founder of the Blockchain Foundation, a couple of days later. The DAO realizing that this might just be a real deal decided to rectify this right away (or soon enough, depending on your perspective) and on June 14th, they do the right thing and propose the fixes for governance approval, in the truly decentralized way.

On June 17th, 2016, the DAO contracts were exploited using a combination of vulnerabilities, including the one concerning recursive calls, that resulted in the transfer of 3.6 million ether, over one-third of what they owned and valued at around $50M then. Too little, too late?

But hey they thought of this, kind of? Timelocks were in place so it meant that they wouldn't actually have access to the funds until a while later. In this amount of time, the Ethereum community was in two minds, was it really fair they got blackhatted out of their crypto? And was it right that they would have to change the immutable nature of blockchain to undo the hack? Eventually, the widespread consensus was for a hardfork, with a relatively small number continuing on the historical chain, now called Ethereum Classic.

But that aside, let's talk about what "reducing the attack surface" actually entails.

Teach a man to fish

We'll start off fairly simple, let's take a potentially vulnerable or safe contract like this:

// DO NOT USE THIS FOR PRODUCTION!
// THERE IS A MAJOR FLAW IN THIS CONTRACT.
pragma solidity ^0.8.6;

struct Bid {
    address account;
    uint256 amount;
}

contract SimpleAuction {
// While this contract does not have anything to transfer except ether
// We will be assuming an asset to transfer, such as an NFT
    address public admin = 0x1987013F26d9fa9a61856dE905668fcF6CAfE0A8;
    mapping(uint256 => Bid) public auctions;
    mapping(uint256 => address) public owners;
    uint256 public NFTTokenId = 0;
    uint256 public revenue = 0;
  
    function makeAuction(uint256 id) external payable {
        Bid memory currBid = auctions[id];
    	require(currBid.account == address(0) && currBid.amount == 0 && msg.value > 0);
        auctions[id] = Bid(msg.sender, msg.value);
        NFTTokenId += 1;
    }
    
    function bid(uint256 id) external payable {
        Bid memory currBid = auctions[id];
    	require(currBid.account != address(0) && currBid.amount != 0  && owners[id] == address(0)); // checks
        require(msg.value > currBid.amount);
        auctions[id] = Bid(msg.sender, msg.value); // effects
        payable(currBid.account).transfer(currBid.amount); // interactions
    }

    function endAuction(uint256 id) external {
	// Transfer our NFT to the winner, what can go wrong?
        require(msg.sender == admin);
	    Bid memory currBid = auctions[id];
        require(currBid.account != address(0) && currBid.amount != 0  && owners[id] == address(0));
        owners[id] = currBid.account;
        revenue += currBid.amount;
    }
}

From the first look, it looks quite nice, looks like we are covering all our bases, always great to follow the best security practices, right? Look, there's even access control on our function to end an auction.

Well... not so much.


Let's think about this from the perspective of an attacker. How many things can they affect? There's the usual msg.sender, msg.value, tx.gasprice, maybe some block number+timestamp logic by colluding with miners, but apart from that, it's only the things that the contract strictly provides.

But Ethereum's intent to ensure that contracts are first-class citizens compared to EOAs makes for some pretty interesting and sometimes dangerous outcomes, but yes, it's only dangerous if we make it so. So what does that entail?

Solidity is a quite extensible language, which means that you have sufficient leeway in how you want to do what, even if that means it is statically and strongly typed. It also provides two very specific functions to make working with contracts easier and losing your ether harder - receive() and fallback(), namely the Receive Ether and Fallback functions. These are special functions invoked if you're sending ether directly to the contract or if there's no other matching function, respectively.

pragma solidity ^0.8.6;

interface Auction {
    function makeAuction(uint256 id) external payable;
}

contract MaliciousBidder {
    address admin = 0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B;
    Auction auction = Auction(address(0)); // address of a deployed Auction contract
    
    receive() payable external {
    	revert();
    }
    
    function makeBid(uint256 NFTTokenId) external payable {
    	auction.makeAuction{value: 1 wei}(NFTTokenId); // we need > 0 wei
    }
}

So, what makes this contract so interesting? Yep, it's the single revert() statement in our receive() function. What this basically means is that every direct ether transfer will be instantly reverted, while quite useful for not losing your ether in an incompatible or disabled contract, this is quite literally the bane for our ever-so-simple SimpleAuction contract.

If you were to map the control flow using this contract, here's how it would look:

  • Call makeBid() with whichever token ID we want (you'll see why this works), along with a 1 wei transfer (the reason this is needed is that our auction contract does not accept etherless bids, for obvious reasons).
  • Our malicious contract calls our auction contract to make an auction with the lowest possible value.
  • This is when the interesting behavior starts. Imagine someone else makes a bid for that particular NFT.
  • Everything goes fine until our auction contracts hits the last line in our bid() function: payable(currBid.account).transfer(currBid.amount)
  • Since it's a direct ether transfer, the receive() function in our malicious contract gets invoked, then subsequently gets reverted, which in turn reverts the entire function call. And what that basically means is that no one else can bid for that particular NFT anymore.

So, if we did choose to systematically mint all NFTs (notice that our makeAuction() function has no access control), we could theoretically get all NFTs by paying a small, small fee of 2^256 wei (+ gas). This is basically a kind of denial of service attack and is well-documented, if you want to read a bit more about this, take a look at the best practices from the folks at ConsenSys.

A very simple solution to this would be simply to use a pull-based payment system, most well-written and audited contracts use pull-based withdrawals now, while it's not so good for UX, it's a much better option in terms of security.

mapping (address => uint256) balances;

function withdraw() external payable {
    payable(msg.sender).transfer(balances[msg.sender]);
    // Or, better:
    // (bool success,) = payable(msg.sender).call{value: balances[msg.sender]}("");
    // require(success);
}

Breaking the bank

In the first post, we wrote our simple bank contract and talked about recursive call vulnerabilities. Re-entrancy attacks have been a problem since the dawn of smart contracts and chances are they will not go away, if I had to hazard a guess, including guards by default in the language would have significant cost and maybe speed implications, which would also explain why it has not been done for so long.

The best part about recursive call vulnerabilities is often how dead simple they are, which also makes it somewhat simple to fix them. If we were to write a contract to exploit our SimpleBank contract from the previous post, all we would need is something like this:

pragma solidity ^0.8.6;

interface UnsafeBank {
    function withdrawTo(address dest) external;
}

contract BankRobber {
    address public admin = 0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B;
    UnsafeBank bank = UnsafeBank(address(0)); // address of our bank
    
    receive() external payable {
    	if (address(bank).balance > 0) {
            bank.withdrawTo(admin);
        }
    }
}

Assuming you provided the initial ether transfer invocation with enough gas, this process would repeat until we drained our bank of all ether, amazing right? So, how do we fix it?

One approach we mentioned earlier in the series was the "checks-effects-interactions" pattern, and the other would be to use re-entrancy guards, essentially function modifiers that detect if a function is invoking itself and mitigate this form of attack. It should be noted that using either one should not be the reason for not implementing the other and you should assess both approaches to see which one is needed where since both have their limitations.

For example, here's how the OpenZeppelin implementation of a re-entrancy guard looks: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.3/contracts/security/ReentrancyGuard.sol It also comes with some handy documentation comments so you can understand how it works.

Parting words

A lot of the legwork while writing smart contracts is simply keeping track of the latest best practices and diligently following them. For starters, the list of best practices from ConsenSys is pretty solid and I would treat it as the minimum recommended reading when writing contracts in production. It is also highly recommended to keep track of breaking changes between major version updates of Solidity and new or deprecated OPCODEs in the EVM since there might be new behavior and interactions that you might not expect.

Last but not the least, keep your eyes and ears about, if something you write looks suspicious. it probably is. I would say that it's always the best to get your code reviewed by at least one or two other humans (or in my case, about five, but you get the gist). Oh, and please follow the style guide, chances are your code reviewers don't appreciate free headaches. 😴