# Findings - Conic Part 1

### 2.1 Critical

#### 1. Wrong conversion from Curve/Convex LP to underlying using Chainlink oracle

##### Description

**Impact**

An attacker can steal from a Conic Pool the amount equal to:

AttackerNotionalAmount * %(Actual Conversion Rate - Chainlink Rate) * ~60%

Given a 0.3% price difference between Curve and Chainlink and 1M USD of funds prepared for the attack, an attacker can steal ~2000 USD risk-free.
These 2000 USD are taken from another pool depositors, they will experience a loss on their withdrawals.

**Attack Flow**

The attack is easier to illustrate by the decreasing underlying Chainlink price by 0.3%. Curve balances remain unchanged, thus the price has not decreased on Curve. As the result we have a difference of 0.3%.

0) Let's have some depositors in the pool:
- UserA deposited 3 000 USD
- UserB deposited 3 000 USD
1) The attacker waits for the 0.3% difference, e.g. a Chainlink transmit
2) ConicPool.deposit(1M USD), from attacker
3) Wait 1 block
4) ConicPool.withdraw(), attacker returns 1 001 200 USD.
5) other depositors withdraw:
- User A goes first, returns 1 940 USD
- UserB goes last, returns 1 933 USD

**Root cause**

In our case described above, `getTotalAndPerPoolUnderlying()` returns `totalUnderlying` as 1 008 000 USD,
when only 1 006 000 USD was deposited.
- https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/contracts/BaseConicPool.sol#L673-L684

It goes to `CurveAdapter.computePoolValueInUnderlying()`
- https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/contracts/adapters/CurveAdapter.sol#L95-L104
It calculates the value of CurveLP tokens, measured in our underlying.
As the underlying price decreases, this value above must increase.

Then, in `withdraw()` we have the following crucial calculation.

underlyingToWithdraw = 
    conicLpIn 
    *
    value deposited in Curve/Convex, measured in underlying
    /
    conicLpTotalSupply;

The second argument is a problem. It is calculated purely with oracles. So, it means the following false statement:

"If underlying price depreciated,
ìt means that Conic investments in underlying appreciated. 
Thus Conic should withdraw more underlying from Curve/Convex."

This statement requires that Curve is immediately ready to withdraw more underlying for depositors, as Chainlink price updates underlying price. This is not necessarily true.

As a result, ConicPool calculates that Curve can withdraw 1 008 000 of underlying, when in fact (on Curve) it is only 1 006 000.
Thus our attacker deserves to withdraw more underlying - 1 002 000 with only 1 000 000 invested.

This pool loss will be distributed among other depositors.

##### Recommendation

When converting between Curve/Convex LP value and underlying, Chainlink does not necessarily indicate true estimation for backed underlying.
It is better to take into account real underlying backed by these LPs and follow Curve/Convex calculations.

### 2.2 High

Not found

### 2.3 Medium

#### 1. ChainlinkOracle fails to return WBTC price, pools with WBTC are not supported

##### Description

`oracle.getUSDPrice(wbtc)` returns the error `"token not supported"`
As the result, it is impossible to add Curve pools with WBTC.

But, Conic mentions pools with WBTC in tests:
https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/test/ConicTest.sol#L39

...
address internal constant REN_BTC = 0x93054188d876f558f4a66B2EF1d97d16eDf0895B;
address internal constant BBTC = 0x071c661B4DeefB59E2a3DdB20Db036821eeE8F4b;
...

Also, there are many Curve pools with WBTC, some of them supported by Convex as well.
BTC is a reserved denomination with the address: `0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB`
- https://github.com/smartcontractkit/chainlink/blob/55c7baa1181aaa04026b4a1043117596c4e31e5a/contracts/src/v0.8/Denominations.sol#L7

So WBTC price feed can be received only by:

WBTC = 0x514910771AF9Ca656af840dff83E8264EcF986CA
BTC = 0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB
wbtcbtc = _feedRegistry.getFeed(WBTC, BTC)

It will be WBTC/BTC price; then it must be adjusted to BTC/USD price:

BTC = 0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB
USD = address(840)
btcusd = _feedRegistry.getFeed(WBTC, BTC)

##### Recommendation

We recommend implementing additional calculations in `ChainlinkOracle._getPrice()` and `ChainlinkOracle.isTokenSupported()` in order to support Curve Pools with WBTC.

#### 2. GenericOracle public frontrun for initialize()

##### Description

The first call of `GenericOracle.initialize()` can be made by anyone.
https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/contracts/oracles/GenericOracle.sol#L20-L24
As a result, anyone can frontrun the official initialize() call and set malicious oracles, which is extremely dangerous for the whole project.

##### Recommendation

We recommend implementing onlyOwner modifier, the same way as for `initialize()` functions in `Conroller` and`RewardManager`.

#### 4. Chainlink min&max price not checked

##### Description

Some chainlink aggregators have min and max price. The price of an asset cannot go outside the range of min and max price.
https://docs.chain.link/data-feeds#check-the-latest-answer-against-reasonable-limits
`When the reported answer is close to reaching reasonable minimum and maximum limits ... it can alert you to potential market events.`
These prices can be found here:
https://github.com/smartcontractkit/libocr/blob/9e4afd8896f365b964bdf769ca28f373a3fb0300/contract/OffchainAggregator.sol#L56-L59

##### Recommendation

According to Chainlink's recommendation:
`Configure your application to detect and respond to extreme price volatility or prices that are outside of your acceptable limits.`

### 2.4 Low

#### 1. Restriction for multiple mints/burns in one block can be bypassed

##### Description

If a transfer amount is below `minimumTaintedTransferAmount`, the following is possible:
1) UserA mints LP tokens
2) UserA transfers LP tokens to UserB
3) UserB burns LP tokens

https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/contracts/LpToken.sol#L72-L75
Thus, UserA can transfer the full LP balance with multiple transfers and bypass the restriction.

##### Recommendation

We recommend tainting every LP token transfer.

#### 2. Extra reward tokens can be stolen by anyone if no oracle indicated

##### Description

Extra tokens with no oracle will have `_minAmountOut` set to zero. It will allow anyone to steal these tokens when `RewardManager` tries to swap these tokens to ETH.
https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/contracts/RewardManager.sol#L504-L506

##### Recommendation

This is a known issue and developers stated that they accept the risk.
https://github.com/ConicFinance/protocol/blob/7a66d26ef84f93059a811a189655e17c11d95f5c/contracts/RewardManager.sol#L501-L503
But we recommend a better solution - to take into account `price0CumulativeLast()` and `price1CumulativeLast()` that would make the attack less easy.

#### 3. No max length for pools indicated

##### Description

Ethereum gaslimit will not allow Conic Pool operation if too many pools are added.
Marginal gas consumption for additional pools:
- ~ 820 000 for deposits
- ~ 1 300 000 for withdrawals
So, the approximate max length is 22 pools.

##### Recommendation

We recommend indicating some desired limit for the length of pools.