Bancor V2 AMM Security Audit

1 Executive Summary

This report presents the results of our engagement with Bancor to review version 2 of the Bancor Liquidity pool. The initial review was conducted over the course of two weeks, from July 15th to July 26th, 2020 by Bernhard Mueller and Sergii Kravchenko. A total of 15 person-days were spent.

During the first week, the auditors familiarized themselves with the existing Bancor codebase as well as the novel concepts introduced with version 2 of the liquidity pool. They assessed the code quality and checked for basic vulnerabilities, such as integer overflow bugs and improper access permissions, and performed a preliminary analysis with regards to the business logic. During the second week the auditors focused on the behavior of the new liquidity pool model and its incentive mechanisms.

Several issues were discovered which were subsequently addressed by the Bancor team. These fixes were reviewed over the course of two days, July 28th and July 29th (a total of 4 person-days).

2 Scope

The initial 2-week review was conducted on a private Github repository while the codebase was still under development. The list of files covered in the audit can be found in the Appendix.

The review of the fixes occured on a recent commit on the public Bancor contracts repository.

The total time budget of 3 person-weeks allotted to the main review phase was limited considering the complexity of the system. Additionally, the code underwent several changes throughout the audit. The review was performed on a best-effort basis with a focus on covering as much ground as possible given the time available.

The mitigations provided by the Bancor team were reviewed over the course of 4 person-days, which was sufficient to confirm that the mitigations are somewhat effective without exhaustively assessing their overall impact on the system. Due to its high complexity, the system needs to be tested extensively under real-world conditions.

2.1 Objectives

Together with the the Bancor team, we identified the following priorities for our review:

1. Identify known vulnerabilities particular to smart contract systems, as outlined in our Smart Contract Best Practices, and the Smart Contract Weakness Classification Registry;
2. Review the novel AMM mechanisms as well as the correctness of the code implementing the new formulas;
3. Evaluate whether the system is consistently implements the intended functionality and identify ways of gaming the system.

2.2 Key Observations

The code is generally of good quality and well-readable. The new formulas introduced with v2 come with extensive test coverage and have been shown to behave correctly under regular market conditions. That said, given the complexity of the system it is difficult to assess whether the results incentives and fee mechanisms will produce the desired results in a real-world scenario.

In the initial phase of the audit, the auditors demonstrated several ways of extracting value from the pool. For instance, arbitrageurs could frontrun Oracle updates and make conversions that took the future rebalancing into account (6.4). In an early version, this arbitrage could be performed atomically by trading at a stale rate, triggering the Oracle update and swapping in the same transaction (6.1). Users could also minimize slippage and fees by atomically adding and removing large amounts of liquidity (e.g. provisioned via a flash loan) before and after a conversion (see 6.2 and 6.3).

Several mitigations for these issues were put in place in the final version that make these attacks more difficult and to perform and riskier for the attackers. However, the mitigations also increased the complexity of the system.

Fundamentally, the weight rebalancing mechanism creates an incentive to align the balance of the primary reverve (i.e., the reserve containing an arbitary token we’ll refer to as “XYZ”) such that deficits in that reserve are compensated at the cost of liqudiity providers (LPs) who stake in the secondary reserve (which usually contains BNT). A dynamic fee calculation mechanism has been added in the mitigation phase to counterbalance the risk of BNT reserve balance permanently falling below the staked balance. This mechanism charges users additional swap fees whenever the actual BNT balance is in deficit (assuming the XYZ reserve is balanced). These additional fees are directed to decrease the deficit. Ultimately however, some risk remains that the pool contains insufficient reserves and not all LPs can withdraw their stake. This risk primarily affects BNT LPs, but XYZ reserves may also have insufficient funds if the total reserves deficit is large enough.

LPs should be adequately informed about the financial risks associated with providing liquidity to either reserve. It is also recommended to carefully monitor the system in production with limited liquidity.

3 Recommendations

• Carefully review the issues and code quality recommendations described in this report.

• Create an exhaustive threat model describing all edge cases and possible attack vectors with respect to variants of frontrunning and/or gaming the incentive structure created by the Bancor V2 formula. Some potential attacks have been outlined in this report but the analysis is not exhaustive. Clearly inform liquidity providers about the risks associated with staking tokens in the pool.

• Once the code and formula is finalized, it would be ideal to perform a detailed audit of the overall system architecture in combination with a bug bounty program. Notably, Bancor is already planning to launch with a beta pilot that limits the maximum amount of liquidity per pool.

4 System Overview

Existing liquidity pools, including Bancor V1, require liquidity providers to add determinate amounts of each of the underlying assets to the pool. This exposes providers to a downside risk called impermanent loss: If the relative external market price of the assets diverges from the price when liquidity was provided, the value of the assets the provider can withdraw may be less than the combined value when the liquidity was added.

Bancor V2 seeks to eliminate the risk of impermanent loss by allowing users to provide liquidity with up to 100% exposure to a single ERC20 token. This is achieved by dynamically adjusting the relative weights of the reserves based on an external Oracle. With every “rebalancing”, the weights are adjusted such that the pool represents the external price ratio between the token in the primary reserve (which contains an arbitrary ERC20 token, referred to as XYZ in technical documentation) and the token in the secondary reserve (usually BNT).

An additional feature of V2 is “Virtual Amplification”, a multiplier that virtually inflates the staked reserves to allow for lower slippage in conversions. The idea is to allow for more and larger conversions to occur on the pool, thus generating more fees for liquidity providers.

4.1 Actors

The relevant actors are listed below with their respective abilities:

Liquidity pool owners can:

• Update & enable the conversion whitelist. When enabled, only addresses that are whitelisted are allowed to use the converter.

• Set the conversion fee (bound by the max conversion fee set during construction).

• Withdraw any ERC20 tokens held by the anchor. In v2, the anchor is an instance of PoolTokensContainer that contains the two pool tokens.

• In the case of an upgrade, the pool owner temporarily transfers ownership to the Bancor upgrader smart contract, which - once granted ownership - has the additional ability to withdraw tokens and Ether from the pool (in order to transfer it to a new contract account).

Pool users can:

• Add or remove liquidity to/from the pool, which results in pool tokens to be minted or burned. Note that while in v1 there is only a single pool token per pool, in v2 each pool is anchored to exactly two pool tokens (one per reserve).

• Convert between the tokens in reserve. Note that the convert() function is only callable via the Bancor Network contract which was not within the scope of this audit.

5 Issues

Each issue has an assigned severity:

• Minor issues are subjective in nature. They are typically suggestions around best practices or readability. Code maintainers should use their own judgment as to whether to address such issues.
• Medium issues are objective in nature but are not security vulnerabilities. These should be addressed unless there is a clear reason not to.
• Major issues are security vulnerabilities that may not be directly exploitable or may require certain conditions in order to be exploited. All major issues should be addressed.
• Critical issues are directly exploitable security vulnerabilities that need to be fixed.

Initial state:
converter TKN balance = 10000000
converter TKN weight = 500000
converter BNT balance = 10000000
converter BNT weight = 500000

        IPoolTokensContainer(anchor).burn(_poolToken, msg.sender, _amount);

        // calculate how much liquidity to remove
        // if the entire supply is liquidated, the entire staked amount should be sent, otherwise
        // the price is based on the ratio between the pool token supply and the staked balance
        uint256 reserveAmount = 0;
        if (_amount == initialPoolSupply)
            reserveAmount = balance;
        else
            reserveAmount = _amount.mul(balance).div(initialPoolSupply);

        // sync the reserve balance / staked balance
        reserves[reserveToken].balance = reserves[reserveToken].balance.sub(reserveAmount);
        uint256 newStakedBalance = stakedBalances[reserveToken].sub(reserveAmount);
        stakedBalances[reserveToken] = newStakedBalance;

converter TKN balance = 100,000,000
converter TKN weight = 500,000
converter BNT balance = 100,000,000
converter BNT weight = 500,000
frontrunner TKN balance = 100,000,000
frontrunner BNT balance = 0
Oracle A rate = 10,000
Oracle B rate - 10,000

converter TKN balance = 101,000,000
converter TKN weight = 500,000
converter BNT balance = 99,000,500
converter BNT weight = 500,000
frontrunner TKN balance = 99,000,000
frontrunner BNT balance = 999,500

converter TKN balance = 99,995,006
converter TKN weight = 498,754
converter BNT balance = 100,000,000
converter BNT weight = 501,246
frontrunner TKN balance = 100,004,994
frontrunner BNT balance = 0

converter TKN balance = 100,000,000
converter TKN weight = 498,754
converter BNT balance = 99,995,031
converter BNT weight = 501,246
frontrunner  TKN balance = 100,000,000
frontrunner BNT balance = 4,969

_to.transfer(address(this).balance);

if (_targetToken == ETH_RESERVE_ADDRESS)

msg.sender.transfer(reserveAmount);

assert(amount < targetReserveBalance);

6 Bytecode Verification

Bytecode-level checking helps to ensure that the code behaves correctly for all input values. In this audit we used Mythx deep analysis to verify a small number of basic properties on the weight rebalancing and conversion functions and to detect conditions that would cause runtime exceptions. MythX uses symbolic execution and input fuzzing to explore a large amount of possible inputs and program states.

Note that the Bancor formula is compiled with solc-0.4.25 / 20,000 optimization passes.

We checked whether the following properties hold for all inputs:

• [P1] Function balancedWeights: Sum of weights returned by must equal MAX_WEIGHT
• [P2a] Function crossReserveTargetAmount: Output amount must not be greater than target reserve balance
• [P2b] Function crossReserveTargetAmount: If reserve balances are equal and source weight < target weight, target amount must be lower than input amount

Note that balancedWeights is known to revert when (t * p) / (r * q) * log( s / t) is not in the range [-1/e, 1/e], where:

• t is the primary reserve staked balance
• s is the primary reserve current balance
• r is the secondary reserve current balance
• q is the primary reserve rate
• p is the secondary reserve rate

The following preconditions were set on the input to reflect realistic input ranges. For balancedWeights:

require(_primaryReserveStakedBalance > 0);
require(_primaryReserveBalance > 0);
require(_secondaryReserveBalance > 0);
require(_reserveRateNumerator > 0);
require(_reserveRateDenominator > 0);
require(_reserveRateNumerator < 10 ** 6);
require(_reserveRateDenominator < 10 ** 6);
require(_primaryReserveStakedBalance <= 10**30);
require(_primaryReserveBalance <= 10**30);
require(_secondaryReserveBalance <= 10**30);

For crossReserveTargetAmount:

require(_sourceReserveBalance > 0);
require(_targetReserveBalance > 0);
require(_sourceReserveBalance <= 10**30);
require(_targetReserveBalance <= 10**30);
require(_sourceReserveWeight + _targetReserveWeight == MAX_WEIGHT);
require(_amount > 0);
require(_amount <= 10**30);

6.1 Results

No violations of the properties tested were found. Our tools also did not identify any cases that would cause the function to revert for the given input ranges.

Appendix 1 - Code Quality Recommendations

A.1.1 Increase test coverage

While test coverage for BancorFormula.sol is high, the tests for ConverterBase.sol and LiquidityPoolV2Converter.sol only reach ~67% of statements and less than 50% of branches. In general we recommend aiming for near-100% test coverage.

Appendix 2 - Files in Scope

This audit focused on the files related to LiquidityPoolV2Converter as well as newly added functions in BancorFormula.

A.2.1 Contracts Description Table

A.2.2 Legend

Appendix 3 - Disclosure

ConsenSys Diligence (“CD”) typically receives compensation from one or more clients (the “Clients”) for performing the analysis contained in these reports (the “Reports”). The Reports may be distributed through other means, including via ConsenSys publications and other distributions.

The Reports are not an endorsement or indictment of any particular project or team, and the Reports do not guarantee the security of any particular project. This Report does not consider, and should not be interpreted as considering or having any bearing on, the potential economics of a token, token sale or any other product, service or other asset. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty. No Report provides any warranty or representation to any Third-Party in any respect, including regarding the bugfree nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third party should rely on the Reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this Report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. CD owes no duty to any Third-Party by virtue of publishing these Reports.

PURPOSE OF REPORTS The Reports and the analysis described therein are created solely for Clients and published with their consent. The scope of our review is limited to a review of Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty.

CD makes the Reports available to parties other than the Clients (i.e., “third parties”) – on its website. CD hopes that by making these analyses publicly available, it can help the blockchain ecosystem develop technical best practices in this rapidly evolving area of innovation.

LINKS TO OTHER WEB SITES FROM THIS WEB SITE You may, through hypertext or other computer links, gain access to web sites operated by persons other than ConsenSys and CD. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such web sites’ owners. You agree that ConsenSys and CD are not responsible for the content or operation of such Web sites, and that ConsenSys and CD shall have no liability to you or any other person or entity for the use of third party Web sites. Except as described below, a hyperlink from this web Site to another web site does not imply or mean that ConsenSys and CD endorses the content on that Web site or the operator or operations of that site. You are solely responsible for determining the extent to which you may use any content at any other web sites to which you link from the Reports. ConsenSys and CD assumes no responsibility for the use of third party software on the Web Site and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.

TIMELINESS OF CONTENT The content contained in the Reports is current as of the date appearing on the Report and is subject to change without notice. Unless indicated otherwise, by ConsenSys and CD.