Thales AMM Smart Contract Audit

# 1. Introduction
iosiro was commissioned by [Thales Market](https://thalesmarket.io/) to conduct a smart contract audit of their Automated Market Maker (AMM). Two auditors conducted the audit from 15 August 2022 until 25 August 2022. A review of changes was performed on 8 September 2022. A total of 19 audit days were used.

This report is organized into the following sections.

* **[Section 2 - Executive summary:](#section-2)** A high-level description of the findings of the audit.
* **[Section 3 - Audit details:](#section-3)** A description of the scope and methodology of the audit.
* **[Section 4 - Design specification:](#section-4)** An outline of the intended functionality of the smart contracts.
* **[Section 5 - Detailed findings:](#section-5)** Detailed descriptions of the findings of the audit.

The information in this report should be used to understand the smart contracts' risk exposure better and as a guide to improving the security posture of the smart contracts by remediating issues identified. The results of this audit reflect the in-scope source code reviewed at the time of the audit.

The purpose of this audit was to achieve the following:

* Identify potential security flaws.
* Ensure that the smart contracts function according to the documentation provided.

Assessing the off-chain functionality associated with the contracts, for example, backend web application code, was outside of the scope of this audit.

Due to the unregulated nature and ease of transfer of cryptocurrencies, operations that store or interact with these assets are considered high risk from cyber attacks. As such, the highest level of security should be observed when interacting with these assets. This requires a forward-thinking approach, which takes into account the new and experimental nature of blockchain technologies. Strategies that should be used to encourage secure code development include:

* Security should be integrated into the development lifecycle, and the level of perceived security should not be limited to a single code audit.
* Defensive programming should be employed to account for unforeseen circumstances.
* Current best practices should be followed where possible.

<a name="section-2"></a>
# 2. Executive summary

This report presents the findings of a smart contract audit performed by iosiro of Thales' AMM.  

The AMM was the third iteration of the Thales Options Protocol. This iteration allowed users to purchase options against an AMM, which improved the liquidity of markets by reducing the reliance of market participants on both sides of an option.

One high risk, one medium risk, and several informational issues were identified during the audit. The high risk issue pertained to the potential for manipulation of option pricing when purchasing options with an alternative collateral to sUSD. The medium risk issue resulted from a discrepancy between the implementations of selling and buying options. The high and medium risk issues had been appropriately addressed by the end of the audit.

The code was found to correctly implement the specification and the unit tests were fairly comprehensive. The readability of the code and adherence to best practices could still be further improved.

<a name="section-3"></a>
# 3. Audit details

## 3.1 Scope

The source code considered in-scope for the assessment is described below. Code from all other files was considered to be out-of-scope. Out-of-scope code that interacts with in-scope code was assumed to function as intended and not introduce any functional or security vulnerabilities for the purposes of this audit.

### 3.1.1 Thales Markets
**Project Name:** Thales AMM <br/>
**Commits:** [0612289](https://github.com/thales-markets/contracts/tree/0612289757c06763e2864332de67c76daabc645c), [894ab23](https://github.com/thales-markets/contracts/tree/894ab23e2bc9b6f24bce3e1a509cc60f9b2e840b)<br/>
**Files:** ThalesAMM.sol

## 3.2  Methodology

A variety of techniques were used in order to perform the audit. These techniques are briefly described below.

### 3.2.1 Code review

The source code was manually inspected to identify potential security flaws. Code review is a useful approach for detecting security flaws, discrepancies between the specification and implementation, design improvements, and high-risk areas of the system.

### 3.2.2 Dynamic analysis

The contracts were compiled, deployed, and tested in a test environment, both manually and through the test suite provided. Manual analysis was used to confirm that the code was functional and to identify security issues that could be exploited.

### 3.2.3 Automated analysis

Tools were used to automatically detect the presence of several types of security vulnerabilities, including reentrancy, timestamp dependency bugs, and transaction-ordering dependency bugs. Static analysis results were reviewed manually and any false positives were removed. Any true positive results are included in this report.

Static analysis tools commonly used include Slither, Securify, and MythX. Tools such as the Remix IDE, compilation output, and linters could also be used to identify potential areas of concern.

## 3.3  Risk ratings

Each issue identified during the audit has been assigned a risk rating. The rating is determined based on the criteria outlined below.

* **High risk** - The issue could result in a loss of funds for the contract owner or system users.
* **Medium risk** - The issue resulted in the code specification being implemented incorrectly.
* **Low risk** - A best practice or design issue that could affect the security of the contract.
* **Informational** - A lapse in best practice or a suboptimal design pattern that has a minimal risk of affecting the security of the contract.
* **Closed** - The issue was identified during the audit and has since been satisfactorily addressed, removing the risk it posed.

<a name="section-4"></a>
# 4. Design specification
The following section outlines the intended functionality of the system at a high level.

## 4.1 AMM

The Thales AMM is an automated market maker for binary options. It allows buying and selling binary options for a given set of markets. A separate market manager contract creates these markets. As an AMM, it permits the trading of options by sourcing liquidity from existing positions, as well as absorbing some of the counterposition in trades through liquidity provided by liquidity providers (initially Thales themself). This exposes the liquidity providers to a degree of risk, but the pricing model is intended to mitigate this.

Market participants primarily buy options using sUSD on Optimism and USDC on Polygon. However, participants on Optimism can opt to purchase options using alternative stablecoins, such as USDT, DAI, or USDC. Using alternative collateral types is made possible by swapping them for sUSD using a SUSD/3CRV Curve pool. Participants are paid in sUSD when they sell their options.

## 4.2 Market Lifecycle

Markets are created for specific assets (e.g., ether, SNX, etc.) with a given strike price and maturity date. Options can be traded up to 24 hours before the maturity date of the market. This constraint is applied to avoid some time-based attacks against the system, such as frontrunning.

The options are binary, meaning they represent whether the asset's actual price is anticipated to be above or below the market's strike price at maturity. Therefore, the options are referred to as UP tokens (asset price will be above strike price) or DOWN tokens (asset price will be below strike price).

After a market has reached maturity, participants can exercise their options. If a participant decides to exercise their options, and their options reflect the correct outcome of the market, they will be able to redeem 1 sUSD for each option of the winning side they own. For example, if a participant held 100 UP tokens, and 50 DOWN tokens, and the asset price is above the strike price at the time of maturity, the participant can exercise their options for a total of 100 sUSD.

## 4.3 Options Pricing

The base price of an option is derived using the Black-Scholes model, which uses various inputs to derive the odds of a specific market being UP or DOWN. These inputs include the asset's current price, the market's strike price, the asset's implied volatility, and the time left to maturity.

The odds obtained from the Black-Scholes model are then used to determine the buy and sell base price for the options of a specific market. The base price is then modified by adding (in the case of buying) or subtracting (in the case of selling) protocol fees. The price will also account for the current exposure of the AMM, which should encourage unskewed markets.

## 4.4 Market Exposure Balancing

When a market participant takes on a position, for example, buying 100 UP tokens for a given market, the AMM will buy 100 DOWN tokens for the same market. At maturity, the participant or the market can exercise their options for an equivalent amount of sUSD. At face value, this strategy puts the AMM at risk of over-exposure to a market that might not resolve in the AMM's favor.

To curb this, the price of buying options that the AMM has already bought is significantly lower, incentivizing buyers to relieve the AMM's exposure. Similarly, if a participant buys options that would increase the risk exposure of the AMM, the price of the options become significantly more expensive.

Participants who sell options to the AMM in the direction it is already exposed receive a low price. Participants who sell options in the opposite direction of the AMM's exposure receive a better price. The discrepancy in pricing encourages participants to sell tokens to the AMM that it requires to burn a portion of its tokens, effectively reducing the exposure of the AMM.

<a name="section-5"></a>
# 5. Detailed findings
The following section includes in-depth descriptions of the findings of the audit.

## 5.1 High risk

No high-risk issues identified during the audit were present at the conclusion of the audit.

## 5.2 Medium risk

No medium-risk issues identified during the audit were present at the conclusion of the audit.

## 5.3 Low risk

No low-risk issues identified during the audit were present at the conclusion of the audit.

## 5.4 Informational  

### 5.4.1 Add admin function validation

Many of the admin functions did not implement any validation on the parameters provided. While the role is trusted, incorrect values may lead to undesirable outcomes. The following functions were affected.
* `setMinimalTimeLeftToMaturity()`
* `setWhitelistedAddress()`
* `setMinMaxSpread()`
* `setSafeBoxData()`
* `setMinMaxSupportedPriceAndCap()`
* `setPriceFeedAndSUSD()`
* `setStakingThalesAndReferrals()`
* `setPositionalMarketManager()`
* `setCurveSUSD()`
* `setCapPerAsset()`

#### Update
The Thales Market team acknowledged the recommendation, but rejected it based on reducing contract size.

### 5.4.2 No validation of Chainlink TTL

The Chainlink TTL was not validated to ensure it was updated within the reported heartbeat. Validation could be added to ensure prices are within the heartbeat values set out in the ([Chainlink Optimism documentation](https://docs.chain.link/docs/optimism-price-feeds/#Optimism%20Mainnet) - click the `Show more details` button).

#### Update
The Thales Market team acknowledged the recommendation and suggested that it may be addressed in a future implementation.

### 5.4.3 Missing event

The `setStakingThalesAndReferrals()` admin function does not emit an event when it is called. Events aid in the visibility of contract state changes. This information can be used on the dApp frontend, and could also be helpful for users.

## 5.5 Closed

### 5.5.1 Option price oracle manipulation (high risk)

#### Description
When purchasing options with alternative collateral to sUSD, Curve Protocol is used to swap the collateral into sUSD. In order to establish the 3CRV/sUSD price, the `get_dy_underlying()` function is used. This function is susceptible to price manipulation in a single block by an attacker with access to a large amount of funds or a sufficiently sized flash loan.

An attacker could deposit a significant amount of `sUSD` into the Curve pool and severely skew the price of the supported collaterals (`USDC`/`USDT`/`DAI`). In doing so, they would be able to buy options for a given market for significantly less in the collateral token amount, but sell them for a higher amount of sUSD. In doing so, the AMM could be drained of its funds.

#### Recommendation
Two potential mitigations are suggested.

1. Use the Curve TWAP for the collateral price to ensure that the sUSD/3CRV price cannot be manipulated in a single block.
2. Validate that the amount of 3CRV assets required to purchase the options is within a specified range of the expected sUSD price. Given that the assets are expected to trade at approximately the same price, and the associated fees with purchasing options, ensuring that the values are within some range of each other (e.g. 95%) will largely reduce the viability of such an attack.

#### Update
The second mitigation was implemented in [894ab23](https://github.com/thales-markets/contracts/commit/894ab23e2bc9b6f24bce3e1a509cc60f9b2e840b#diff-610f1c34a6244526a7b39723079afe81355d0e6acc0141f6f90ed59e3ba898f9).

### 5.5.2 Inconsistent usage of `minSpread` in buy and sell operations (medium risk)

#### Description
When calculating the maximum buy or sell amount for a given market and position, `basePrice` is first checked to be between `minSupportedPrice` and `maxSupportedPrice`. As such, there appears to be an inconsistency between the `basePrice` used in the buy and sell validation. When calculating the maximum buy amount, `minSpread` is first added to `price()` to calculate `basePrice` ([ThalesAMM.sol#L109](https://github.com/thales-markets/contracts/blob/1fea851742005e001294dcbcfdfba707826eea3c/contracts/AMM/ThalesAMM.sol#L109)), but the calculation for `basePrice` does not account for `minSpread` before the range check ([ThalesAMM.sol#L520](https://github.com/thales-markets/contracts/blob/1fea851742005e001294dcbcfdfba707826eea3c/contracts/AMM/ThalesAMM.sol#L520)).

#### Recommendation
When validating the `basePrice` during a buy operation, the `minSpread` term should not be added.

#### Update
The recommendation was implemented in [894ab23](https://github.com/thales-markets/contracts/commit/894ab23e2bc9b6f24bce3e1a509cc60f9b2e840b#diff-610f1c34a6244526a7b39723079afe81355d0e6acc0141f6f90ed59e3ba898f9).

### 5.5.3 Restrict `mint` and `burn` calls (informational)

To reduce the attack surface of the system the `mint()` and `burn()` functions in the PositionalMarket.sol contract should be restricted. No direct attacks were observed, but the exposure of these functions presents an unnecessary risk.

#### Update
Implemented in [894ab23](https://github.com/thales-markets/contracts/commit/894ab23e2bc9b6f24bce3e1a509cc60f9b2e840b#diff-5362e11a5256796a0da53f5c710de9fedc177c9d191c19813d807644c0a06c53).

Secure your system.
Request a service
Start Now