Uniswap audit

We are proud to announce that we have completed an audit of the Uniswap decentralized exchange. To our knowledge this is also the first audit of a major smart contract system written in Vyper.

Uniswap is unique and important not only because of the source code language, but also as one of the fast-growing, and most value holding decentralized systems on the Ethereum blockchain. As a system without a native token, developed through grant funding, it is also a true public good.

Summary

The Uniswap code base was high quality, thoroughly commented, and we were impressed with the audibility of the Vyper language. In our opinion, more developers should consider Vyper for use in their projects.

You can read the complete audit report here. In the rest of this post, we will describe an important undocumented ‘trust issue’. To our knowledge, this issue does not currently place any funds at serious risk, but may in the future.

Description of the issue

Certain apparently benign token implementations, created without malicious intent, could enable a reentrancy attack resulting in the theft of all the ETH and token liquidity in the Uniswap Exchange contract associated with that specific token. To be clear, there is no known risk to the ETH or token liquidity in other token exchanges: don’t worry, your DAI is safe, and any ETH in the DAI exchange is also safe.

The condition required to enable this attack is simple to describe:

• The token makes an external call in the transferFrom function to an attacker-controlled address.

Although uncommon, this condition is completely compatible with the ERC20 standard. In practice, it is most likely to occur within a Security Token, which would typically make a call to the controller or similar 3rd parties, whom you must already trust in order to hold the token. The greatest danger would occur if a call is made to the sender (ie. _from in transferFrom(_from, _to, _value) .

Description of the attack

Let’s imagine a token that has an external call inside the transferFrom() function to the from address (or any other address controlled by the attacker). Here is an example of such a function written in Vyper:

@public
def transferFrom(_from : address, _to : address, _value : uint256(wei)) -> bool:
    _sender: address = msg.sender
    allowance: uint256(wei) = self.allowances[_from][_sender]
    self.balances[_from] = self.balances[_from] - _value
    self.balances[_to] = self.balances[_to] + _value
    self.allowances[_from][_sender] = allowance - _value
    if self.has_sender_callback[_from]:
        assert TokenSender(_from).tokensSent(_to, _value)
    log.Transfer(_from, _to, _value)
    return True

The line TokenSender(_from).tokensSent(_to, _value) makes a call to tokensSent() function of the _from address. It allows the attacker to reenter the Uniswap exchange, and do another Uniswap trade before this transferFrom() function call returns. We are considering the case when reentrancy is made after the token balances are updated. If reentrancy is made before token balances are updated (e.g. ERC-777 token), the stealing algorithm is even easier and requires less funds to steal the liquidity pool.

In Uniswap’s tokenToTokenInput function an attacker can make reentrancy on assert self.token.transferFrom(buyer, self, tokens_sold) and is able to make an additional transaction before first Uniswap trade finishes (ETH from the initial trade is still to be sent in the next line of code).

@private
def tokenToTokenInput(tokens_sold: uint256, min_tokens_bought: uint256, min_eth_bought: uint256(wei), deadline: timestamp, buyer: address, recipient: address, exchange_addr: address) -> uint256:
    assert (deadline >= block.timestamp and tokens_sold > 0) and (min_tokens_bought > 0 and min_eth_bought > 0)
    assert exchange_addr != self and exchange_addr != ZERO_ADDRESS
    token_reserve: uint256 = self.token.balanceOf(self)
    eth_bought: uint256 = self.getInputPrice(tokens_sold, token_reserve, as_unitless_number(self.balance))
    wei_bought: uint256(wei) = as_wei_value(eth_bought, 'wei')
    assert wei_bought >= min_eth_bought
    assert self.token.transferFrom(buyer, self, tokens_sold)
    tokens_bought: uint256 = Exchange(exchange_addr).ethToTokenTransferInput(min_tokens_bought, deadline, recipient, value=wei_bought)
    log.EthPurchase(buyer, tokens_sold, wei_bought)
    return tokens_bought

Let’s look closer into a detailed algorithm of draining funds from the exchange in that case:

1. Assume we have an exchange with a token of equal value to ETH with equally large liquidity pools (100 tokens, 100 ETH)

2. An attacker creates an Exchange for their own token (it will be the second exchange in tokenToToken() transfers) that will receive ETH from the first exchange.

3. According to the Uniswap price function, the attacker can buy 50 ETH for 100 tokens by using the tokenToTokenInput() function.

4. The new liquidity pool should then be (200 tokens, 50 ETH) but since the attacker reenters on assert self.token.transferFrom(buyer, self, tokens_sold) before the ETH was transferred it will still be (200 tokens, 100 ETH) and 50 ETH should be transferred to the second exchange later.

5. While making reentrancy the attacker can buy 49.9999 ETH for about 200 tokens using tokenToEthSwapInput.

6. After that, the liquidity pool should look like (400 tokens, 0.0001 ETH)

7. Now the attacker can buy almost all the tokens for a very small amount of ETH, e.g. buy 399.6 tokens for less than 0.1 ETH and leaving remain liquidity pool look like (0.4 tokens, 0.1 ETH).

This is not a very accurate algorithm because it does not include fees and gas cost, but the logic stays the same.

How did this happen?

The algorithm behind Uniswap is well balanced and elegant but it’s also non-trivial. Its complexity makes it difficult to estimate the damage that reentrancy may cause.

In Uniswap, every token has its own exchange contract. On every exchange, you can be a liquidity provider and/or make trades. At any moment in time, every exchange has X amount of ETH and Y amount of tokens in the liquidity pool. The main principle of implementing trading on the exchange is to ensure that X*Y = invariant. The X*Y value must remain the same before and after every trade done on that exchange.

The invariant should only be changeable when adding/removing funds to/from the liquidity pool. When you add liquidity to the liquidity pool, you should submit both ETH and tokens in the same proportion as the current liquidity pool (X, Y). The price of every trade is automatically calculated in such a way as to keep the invariant the same. Introducing trading fees changes the algorithm slightly, but the main principle remains the same.

The system is “well balanced” in that you can make multiple trades to buy or sell some amount of tokens and then make the opposite trades to sell or buy these tokens back. Afterwards, both the trader and the exchange should have the same amount of tokens and ETH as before the trades (excluding fees and gas cost which we are ignoring at the moment).

According to the X*Y = invariant formula, the more ETH/tokens a trader buys, the more expensive they become. So buying all the remaining ETH in the liquidity pool will cost an almost infinite amount of tokens. On the other hand, if you sell that ETH back afterwards, you will receive the same amount of tokens back. So if the attacker is able to move the price to a very huge number, he can then profit (receiving almost all of the liquidity pool) by moving the price back to a fair price. This is exactly what was used in the attack. Inside the reentrancy transaction we know that the last 50 ETH will be transferred after this transaction, so draining ETH to almost 0 will not cost an infinite amount of tokens. The attacker can drain it to 50.0001 and then 50 ETH will be transferred as a part of the initial transaction. After that when the amount of ETH is close to zero, the attacker can make a trade and receive almost all of the tokens in the liquidity pool.

What types of tokens enable this attack?

External calls inside transfer/transferFrom function can be called either before or after the token balances were changed. This is important, depending on the mentioned condition, there can be two different ways to steal funds from the exchange. Let’s look at some scenarios where reentrancy can be done:

1. If the external call is made in transfer function to the recipient (or any other account controlled by the attacker) before the balances were updated.

2. If the external call is made in transferFrom function to the token spender (or any other account controlled by the attacker) before or after the balances were updated. It’s important to notice that all transferFrom functions are always called with the exchange as the recipient, so callback to the recipient which is common in different ERC20 extensions is not dangerous.

The first example is quite rare, but the second one can be observed in multiple places:

• Every ERC-777 token should have a callback to the spender before the balances are changed and to the recipient after. That allows everyone to make malicious reentrancy to an exchange contract with ERC-777 token.

• Regulated/controlled tokens typically have an external call to the controller to perform additional validation checks before (or rarely after) every transaction. This is done for KYC or other regulatory requirements. A malicious controller contract could be used in such an attack. Although the owner of a security token contract is usually a public entity who is not interested in stealing tokens, this also provides an additional incentive for a hacker to gain access to the owner’s private keys. Additionally, in some regulated tokens, controllers already have the power to transfer any amount of tokens from any address.

Investigation

To ensure that there are currently no such “callback” tokens placing funds at risk in a Uniswap Exchange, we ran an automated analyses using the eveem.org API to identify all tokens exchangeable in Uniswap which have a CALL opcode in the transferFrom function (csv list). We then manually reviewed those tokens to verify that they were not exploitable by an arbitrary address.

Note that Uniswap also allows users to specify their own exchange contracts, which were generated outside of the Uniswap factory, and we have not investigated any of these.

We haven’t found any token that has a worthy liquidity pool that would allow anyone to perform the attack (like in case of ERC-777 tokens). But we have found multiple exchanges with regulated/controllable tokens as described above. Controllers of the tokens already have some power within these tokens and serious level of trust from the users, so the risk of stealing funds is inconsiderable. On the other hand, adding ERC-777 tokens or any other token with a complex logic that may allow reentrancy from an arbitrary user is not safe and may lead to loss of funds.

Remediation

We recommend adding a mutex to all trading functions in order to prevent reentrancy. That would slightly increase the gas cost of transactions but will make the system more secure toward tokens with more complex structure.

All posts