✅Demystifying Exploitable Bugs in Smart Contracts

7 categories of machine unauditable bugs

Intro

"Demystifying Exploitable Bugs in Smart Contracts" is a paper written by @i2huer et al. for ICSE 2023. Authors looked through many audit reports on Code4rena and Immunefi-like real-world exploits, and categoried "machine unauditable" bugs into 7 categories.

Usually we talk about "bug types" such as reentrancy, unsafe external call, delegatecall storage misalignment, etc, but we don't talk about "bug categories". In this paper authors listed 7 bug categories in smart contracts:

(C1) price oracle manipulation
(C2) erroneous accounting
(C3) ID uniqueness violations
(C4) inconsistent state updates
(C5) privilege escalation
(C6) atomicity violations
(C7) contract implementation specific bugs

These 7 categories represent a higher level of abstraction.

Repo:

This is the paper:

This is the paper's supplemental material:

This supplementary material contains a few vulnerable contract samples that the paper itself does not include. Not sure why, but I will use these samples since I enjoy learning new things by examples.

MUBs vs. MABs

Machine Unauditable Bug (MUB) means exploitable bug that cannot be detected with existing tools.
More than 80% of the exploitable bugs are MUBs.
Price oracle manipulation and privilege escalation are the most common bugs in real-world exploits.
Accounting error is the most common bug in audit contests.
In contrast, if a bug can be detected with existing tools, then it is called a Machine Auditable Bug (MAB).

Statistics

Code4rena

The most popular and lucrative categories on Code4rena are:

Lending
- Lending projects facilitate the borrowing and lending of assets between users. Lenders deposit their assets into the project, earning interest, while borrowers borrow assets by providing collateral.
Dexes
- Dex projects provide a platform for users to exchange assets in a decentralized manner. Uniswap, which we have discussed in detail in the main text, is one of the most well-known Dex projects.
Yield
- Yield projects reward users for staking their funds. Users deposit their funds into the project, and the project invests the funds in various other opportunities. All users share in the profits of the investment, based on their staking shares.
Services
- Service projects offer essential functionalities that can be utilized by other DeFi projects. For example, governance voting is a popular DeFi service, as is the tokenization of seed investors’ and founders’ funds.
Derivatives
- Derivative projects, like their traditional finance counterparts, derive their value from the performance of an underlying entity. Futures and options are two common derivative products in DeFi.

Real-world exploits

"Findings"

Authors list 10 "findings" during their research. They are:

Although the DeFi community has heavily invested on protecting their products, the current supply of tools and human auditor resources have not met the demand.
Existing techniques rely on simple and general oracles or hand-coded ones that are project specific. Such oracles may not be sufficient for functional bugs in general.
A large portion of exploitable bugs in the wild (i.e., 80%) are not machine auditable.
Majority of exploitable bugs are difficult to find.
There are no obvious differences between audit difficulty distributions of MABs and MUBs.
MUBs can be classified to 7 categories, with 85% belonging to categories C1-C6 that are not project specific.
Different types of MUBs have different popularity, with accounting errors (C2) and price oracle manipulation (C1) most popular in the Code4rena bugs and the real exploits, respectively. Auditing is particularly effective in preventing certain bugs such as accounting errors.
Different types of MUBs have different auditing difficulties, with price oracle manipulation and ID uniqueness violation bugs the hardest and the easiest, respectively.
Different kinds of DeFi projects tend to be prone to different types of MUBs.
Five out of the seven MUB categories (accounting for 60% of MUBs), namely, all except (C2) accounting errors and (C7) implementation specific bugs, have general abstract models which may serve as oracles for future automated tools.

Personally I think finding 6, 9, 10 are innovative:

Finding 6 -> Most bugs have universality
Finding 9 -> Auditors should focus on different bug categories for different types of projects
Finding 10 -> Accounting errors and implementation specific bugs are the future.

MABs

MAB categories:

I am not very interested in MABs at this moment. Usually developers would run common tools before turning in the code for audit test. It means the MABs left in the code are the difficult ones. I don't think you can easily find them unless you have customized tools.

MUBs

Authors categoried MUBs into 7 categories. This is the essence of this paper, stay focused.

(C1) Price Oracle Manipulation -> most common in real-world exploits

This bug happens when price oracle does not return correct price and lend to asset loss.

AMM is oftentimes vulnerable to this bug. For example, Uniswap has an official API for querying prices but it has high gas cost. For saving gas, developers tend to implement their own queries.

Before diving into the bug, let's review Uniswap's swap() function:

contract UniswapV2Pair {
    IERC20 token0;
    IERC20 token1;
    uint reserve0;
    uint reserve1;

    function swapToken0ForToken1(uint amount1Out, address to) external {
        token1.transfer(to, amount1Out);

        IUniswapV2Callee(to).uniswapV2Call();

        uint balance0 = token0.balanceOf(address(this));
        uint balance1 = token1.balanceOf(address(this));

        uint amount0In = balance0 - (reserve0 - amount0Out);
        uint balance0Adj = balance0 * 1000 - amount0In * 3;

        require(balance0Adj * balance1 >= reserve0 * reserve1 * 1000, "insufficient funds transferred back");

        reserve0 = balance0;
        reserve1 = balance1;
    }
}

Carbon version of the code:

amount0In * 3 represents a contract fee of 0.3%. A multiplier 1000 is used here to make 0.003 * 1000 = 3. By doing so we convert floating point computation into integer computation.

In the require statement, >= is used because the > case benefits the protocol. User can transfer more tokens into the contract than needed and the contract will take that.

You should be familiar with rest of the logic in this swap function.

The following vulnerable contract comes from Deus Finance. This contract suffered from price oracle manipulation that caused a loss of $3.1 millions:

contract LendingContract {
    IERC20 public WETH;
    IERC20 public USDC;
    IUniswapV2Pair public pair; // USDC - WETH
    // debt --> USDC, collateral --> WETH
    mapping(address => uint) public debt;
    mapping(address => uint) public collateral;

    function liquidate(address user) external {
        uint dAmount = debt[user];
        uint cAmount = collateral[user];
        requrie(getPrice() * cAmount * 80 / 100 < dAmount, "the given user's fund cannot be liquidated");
        address _this = address(this);
        USDC.transferFrom(msg.sender, _this, dAmount);
        WETH.transferFrom(_this, msg.sender, cAmount);
    }

    function getPrice() view returns (uint) {
        return USDC.balanceOf(address(pair)) / WETH.balanceOf(address(pair));
    }
}

Carbon version of the code:

The collateral factor of this protocol is 80%. If user's debt exceeds collateral * 80%, then the collateral will be liquidated.

Once liquidation condition is met, a user can call liquidate() to pay for another user's debt and get his/her collateral. Inside this function, getPrice() is called as price oracle. The developer decided to use this customized function instead the official Uniswap API.

Here is an attack scenario:

Bob (victim) deposits 100 WETH as collateral and borrows 100,000 USDC.
Assume the current price of WETH is $4,000 and the Uniswap AMM holds 100 WETH and 400,000 USDC.
Bob's position is healthy at this stage since his debt is 100,000 USDC and his collateral worths 400,000 USDC.
Alice (adversary) encapsulates the following 5 operations within a single malicious transaction:
1. Borrow 100 WETH flash loan. Flash loan attack works because the entire attack happens within a single transaction.
2. Exchange 100 WETH for 200,000 USDC through Uniswap. Now AMM has 200 WETH and 200,000 USDC, therefore the AMM is still balanced since 100 * 400000 == 200 * 200000. Note that the real-world WETH price is $4,000 but the WETH price in the AMM becomes $1,000. Bob's collateral suddenly devalued a lot.
3. Call liquidate(bob). This is possible because Bob's debt is 100,000 USDC and his collateral worths 100,000 USDC. This ratio exceeds the 80% collateral factor, hence his collateral is liquidable. By paying 100,000 USDC, Alice gets 100 WETH that worths 400,000 USDC in the real world. She makes $300,000 profit here.
4. Exchange 200,000 USDC for 100 WETH. This would undo operation 1 and rebalance the AMM. In this attack, Alice made $300,000 profit with 0 cost.
5. Repay flash loan debt.

To prevent price oracle manipulation, most on-chain DEXes provide manipulation-resistant APIs for price queries. Time-weighted average price (TWAP) is the most common solution nowadays. It is a pricing algorithm that calculates the average price of an asset over a set period. It provides great resistance against flash loans. Recall that a flash loan has to happen within a single transaction and hence the time weight of its manipulated price is 0.

(C2) Erroneous Accounting -> most common in audit contests

The following is a code snippet of the LFW ecosystem, which has been exploited and lost $0.21 millions:

function swap(uint amount1Out, address to) external {
    token1.transfer(to, amount1Out);
    IUniswapV2Callee(to).uniswapV2Call();

    uint balance0 = token0.balanceOf(address(this));
    uint balance1 = token1.balanceOf(address(this));
    uint amount0In = balance0 - (reserve0 - amount0Out);
    uint balance0Adj = balance0 * 10000 - amount0In * 22;
    require(balance0Adj * balance1 >= reserve0 * reserve1 * 1000, "insufficient funds transferred back");
    reserve0 = balance0;
    reserve1 = balance1;
}

Carbon version of the code:

User would call swap() to swap token0 for token1:

token0 ----> swap() ----> token1

The first input amount1Out means how much token1 user wants to swap for, the second input to is user's receiving address.

amount0In * 22 represents a 0.22% contract fee. When calculating balance0Adj, developer used 10000 as multiplier to avoid expensive floating point computation. Since 0.22% = 0.0022, we need 10000 as multiplier so that 0.0022 * 10000 = 22.

However, in the require statement, the multiplier was mistakenly set to 1000. This leads to pricing error. How? Let's do the math:

   balance0Adj * balance1 >= reserve0 * reserve1 * 1000
=> (balance0 * 10000 - amount0In * 22) * balance1 >= reserve0 * reserve1 * 1000
=> (balance0 * 10 - amount0In * 0.022) * balance1 >= reserve0 * reserve1

If we ignore the contract fee, the formula is simplified to:

   balance0 * 10 * balance1 >= reserve0 * reserve1
=> 10 * (balance0 * balance1) >= reserve0 * reserve1

It indicates that the attacker only needs to pay 1/10 of expected token0 to swap for token1.

(C3) ID Uniqueness Violations -> common in audit contests

This vulnerable code snippet comes from the Foundation contest on Code4rena:

contract NFTMarketReserveAuction {
    mapping(address => mapping(uint => uint)) auctionIds;
    mapping(uint => ReserveAuction) idAuction;
    uint auctionId;

    function createReserveAuction(address nftContract, uint tokenId) external ... {
        auctionId++;
        _transferToEscrow(nftContract, tokenId);
        auctionIds[nftContract][tokenId] = auctionId;
        idAuction[auctionId] = NewAuction(msg.sender, ..., tokenId, ...);
        ...
    }

    function _transferToEscrow(address nftContract, uint tokenId) internal ... {
        uint auctionId = auctionIds[nftContract][tokenId];
        if (auctionId == 0) { // NFT is not in auction
            super._transferToEscrow(nftContract, tokenId);
            return;
        } ...
    }
}

Carbon version of the code:

A NFT seller would call createReserveAuction() to start an auction on the NFT to be sold. This function takes two inputs, the first input nftContract is the address of the NFT being sold, and the second input tokenId is the ID of the "currency" the seller want to sell the NFT for.

Within createReserveAuction(), _transferToEscrow() is called. This internal function looks up the auction ID and determines if the ID is 0. If it is, super._transferToEscrow() will be called and a new auction will be initialized.

However, note that the uniqueness of nftContract and tokenId is not guaranteed. That means createReserveAuction() can be called with the same input for multiple times.

Here is an attack scenario:

Attacker calls createReserveAuction() twice with the same nftContract and tokenId.
The first invocation is intended. It correctly transfers NFT and creates a new auction.
The second invocation is unintended. Although the check in _transferToEscrow() would fail, but it does not revert. A new auction will still be created. Note that this new auction is empty because nothing was transferred to it.
At this moment there are 2 auctions for the same NFT. The 1st auction actually contains the NFT and the 2nd auction contains nothing.
Attacker cancels the 1st auction and gets the NFT back. Bidders are still bidding in the 2nd auction.
In the end a bidder becomes winner, but the NFT transfer will revert since the auction is empty. The winner's money is stuck in the protocol and there is no way to withdraw.

The root cause of this bug is the uniqueness of nftContract-tokenId pair is not guaranteed. To fix it, developers should add a check for duplicated auctions.

(C4) Inconsistent State Updates

This vulnerable contract comes from Sushi Trident contest phase 2 on Code4rena:

contract SushiTrident {
    uint128 internal reserve0;
    uint128 internal reserve1;

    function burn(bytes calldata data) public override lock returns (...) {
        (..., uint128 amount, address recipient...) = abi.decode(data, (int24, int24, ..));
        // calculates amounts of each reserve to be returned
        (uint amount0, uint amount1) = _getAmountsForLiquidity(..., amount);
        // calculate fees to burn from amounts to burn
        (uint amount0fees, uint amount1fees) = _updatePosition(msg.sender, ..., -amount);
        ...
        reserve0 -= uint128(amount0fees);
        reserve1 -= uint128(amount1fees);
        // returns the reserve tokens
        _transferBothTokens(recipient, amount0, amount1, ...);
        ...
    }
}

Carbon version of the code:

This is a burn function inside an AMM contract. A user would call burn() to burn LP token and get the ERC20 token plus interest back. In this function a portion of both reserves should be transferred to the user.

It is easy to see the error:

reserve0 -= uint128(amount0fees);
reserve1 -= uint128(amount1fees);

Here the portion that is transferred to the user is not handled, only the fees are deducted.

The fix for this code snippet:

reserve0 -= uint128(amount0fees);
reserve0 -= uint128(amount0);
reserve1 -= uint128(amount1fees);
reserve1 -= uint128(amount1);

(C5) Privilege Escalation

This is a rewritten version of an anonymized real-world contract:

contract Vote {
    struct Proposal {
        uint160 sTime;
        address newOwner;
    }

    IERC20 votingToken;
    address owner;
    Proposal proposal;

    function propose() external {
        require(proposal.sTime == 0, "on-going proposal");
        proposal = Proposal(block.timestamp, msg.sender);
    }

    function vote(uint amount) external {
        require(proposal.sTime + 2 days > block.timestamp, "voting has ended");
        votingToken.transferFrom(msg.sender, address(this), amount);
    }

    function end() external {
        require(proposal.sTime != 0, "no proposal");
        require(proposal.sTime + 2 days < block.timestamp, "voting has not ended");
        require(votingToken.balanceOf(address(this)) * 2 > votingToken.totalSupply(), "vote failed");
        owner = proposal.newOwner;
        delete proposal;
    }

    function getLockedFunds() external onlyOwner {
        ...
    }
}

Carbon version of the code:

User can propose himself/herself as owner candidate by calling propose(). A new proposal will be created. There can only be one on-going proposal.

Other users call vote() to vote using voting tokens. The proposal lasts for 2 days.

When the voting phase ends, end() will be called to set new owner. The voting token collected during vote() calls must exceed 50% of the total supply.

The implementation looks legit, but there exists an unexpected call sequence together with flash loan attack that would result in privilege escalation. Here is how:

Alice proposes herself via propose().
When the deadline proposal.sTime + 2 days is approaching, Alice creates a single transaction that bundles the following operations:
1. Flash loan a lot votingToken from protocol's AMM contract.
2. Call votingToken.transferFrom() to send voting token to the contract directly -> this is the same as calling vote() without actually calling it, so that the require statement is bypassed.
3. Call end() to become owner.
4. Get voting tokens back by calling getLockedFunds().
5. Pay off flash loan debt.

(C6) Atomicity Violations -> least common

A real-world vulnerability in PancakeSwap Lottery contract:

contract Lottery {
  // user address -> lottery id -> count
  mapping (address => mapping(uint64 => uint)) public tickets;
  uint64 winningId; // the winning id
  bool drawingPhase; // whether the owner is drawing

  // invoked every day to reset a round
  function reset() external onlyOwner {
      delete tickets;
      winningId = 0;
      drawingPhase = false;
  }

  function buy(uint64 id, uint amount) external {
      require(winningId == 0, "already drawn");
      require(!drawingPhase, "drawing");
      receivePayment(msg.sender, amount);
      tickets[msg.sender][id] += amount;
  }

  function enterDrawingPhase() external onlyOwner {
      drawingPhase = true;
  }

  // id is randomly chosen off-chain, i.e., by Chainlink
  function draw(uint64 id) external onlyOwner {
      require(winningId == 0, "already drawn");
      require(drawingPhase, "not drawing");
      require(id != 0, "invalid winning number");
      winningId = id;
  }

  // claim reward for winners
  function claimReward() external {
      require(winningId != 0, "not drawn");
      ...
  }

  function multiBuy(uint64[] ids, uint[] amounts) external {
      require(winningIds == 0, "already drawn");
      uint totalAmount = 0;
      for (int i = 0; i < ids.length; i++) {
          tickets[msg.sender][ids[i]] += amounts[i];
          totalAmount += amounts[i];
      }
      receivePayment(msg.sender, totalAmount);
  }
}

Carbon version of the code:

The business model contains 3 operations:

buying tickets
drawing winners
claiming prizes

Among these 3 operations, the "drawing winners" operation is vulnerable. The correct way of implementing it:

enterDrawingPhase() -> this function should be called in a transaction to add a lock
draw() -> this function should be called in another transaction and it should require that lock

We can see that the buy() function is correctly implemented since it has a require statement for the lock:

require(!drawingPhase, "drawing");

However, the contract also has a multibuy() function acting as a batch-buy version of buy(). This function does not require the lock.

Here is an attack scenario:

The protocol enters drawing phase and draw() is called.
Attacker observes the winning ticket ID in the mempool.
Attacker calls multibuy() to batch-buy winning tickets. This is doable since multibuy() does not require the drawingPhase lock.

This is a typical front running attack.

(C7) Contract Implementation Specific Bugs

Here is a redacted version of Cartel contract containing an accounting bug that leads to $560,000 bounty:

contract ERC20 {
  // owner => spender => amount
  mapping (address => mapping (address => uint256)) internal _allowance;

  function _approve(address owner, address spender, uint256 allowance) internal {
      _allowances[owner][spender] = allowance;
  }

  function transferFrom(address from, address to, uint256 amount) external {
      require(_allowances[from][msg.sender] >= amount);
      _approve(from, msg.sender, _allowances[from][to] - amount);
      _transfer(from, to, amount);
  }
}

Carbon version of the code:

This line is problematic:

_approve(from, msg.sender, _allowances[from][to] - amount);

It should be:

_approve(from, msg.sender, _allowances[from][msg.sender] - amount);

This bug is kind of sneaky because _allowances[from][to] sounds like a legit syntax. However, imagine that from, to, and msg.sender are three different people:

from: Alice
to: Bob
msg.sender: Eve

Since _allowances[from][to] has nothing to do with msg.sender, abstractly, Eve is interfering with Alice and Bob's business.

Here is an attack scenario:

Alice approves Bob 10 tokens. Now _allowances[alice][bob] = 10.
Eve calls transferFrom(alice, bob, 0). That means "transfer 0 token from alice to bob".
Inside transferFrom(), _approve(from, msg.sender, _allowances[alice][bob] - 0) is executed.
That is equivalent to _allowances[from][msg.sender] = _allowances[alice][bob] - 0. Here Eve is getting allowance for free.
Eve now has 10 tokens allowance from Alice, so she can steal those tokens with a call transferFrom(alice, msg.sender, 10).

This bug survives several rounds of audit. You can tell that it is hard to find because it is an accounting error that won't be caught by automated tools.

PreviousVulnerabilities SoK NextBlockchain Hacking Techniques 2022 Top 10 - Todo

Last updated 1 year ago