Ichi

Quantstamp30 Apr 2021Files named in the report

Audit

Quantstamp reviewed ICHI on 30 Apr 2021: 29 findings, 0 critical; 21 marked fixed, 8 acknowledged.

No open critical findings

Open the report Open ICHI’s security record

Report hosted on certificate.quantstamp.com.

Trustblock does not review code.

Findings29

Statuses are the security firm’s own, from its report.

Severity	Acknowledged	Fixed
Critical	0	0
High	0	4
Medium	1	3
Low	7	14

MediumPrivileged Roles and OwnershipAcknowledged

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. In particular: 1. Owners of `Arbitrary` strategy contracts can transfer any tokens anywhere. It is also expected that they will retrieve underlying positions manually before `_closeAllPositions` is invoked. 1. When a user mints oneTokens, they can provide both collateral and member tokens, however when redeeming, only collateral tokens may be obtained. It must be ensured by the contract owner and underlying strategy that assets are used in a principle-preserving way. If the owner/strategy does not preserve the principle, the initial holders will indeed receive a 1-to-1 value exchange until the collateral reserve is depleted. Holders that redeem last will find themselves with _valueless oneTokens_ that cannot be redeemed anymore - not even at the minting ratio of 90%. 1. Owners of `OneTokenV1` can set redemption and minting fees up to 100%.

Low`maxOrderVolume` is always `INFINITE`Acknowledged

In the function `getMintingRatio4`, `maxOrderVolume` is set to `INFINITE` in all paths. It is not clear if this is correct or not.

Low`setStrategyAllowance` misses funds that are currently deployedAcknowledged

The `setStrategyAllowance` functions computes the current holdings as the balance of the strategy contract: `IERC20(token).balanceOf(a.strategy);`. This does not include the holdings that are already used in a different contract as part of the strategy. This is also susceptible to being front-run with a transaction that moves the funds to a different contract in the strategy but as strategy funds are controlled by governance, this will most likely not be an issue.

LowClone-and-OwnAcknowledged

The clone-and-own approach involves copying and adjusting open source code at one's own discretion. From the development perspective, it is initially beneficial as it reduces the amount of effort. However, from the security perspective, it involves some risks as the code may not follow the best practices, may contain a security vulnerability, or may include intentionally or unintentionally modified upstream libraries.

LowStub volatility valuesAcknowledged

The volatility of any Uniswap pair is reported as zero, even though price fluctuations may exist.

LowUnclear volatility computation in composite oraclesAcknowledged

The function `read` states that "volatility is calculated by the final oracle". It is not clear why this is only a function of the volatility of the final oracle. If there is volatility in each of the interim tokens, this would influence the resulting `amountOut`.

LowUnclear why `init` may be called multiple timesAcknowledged

The docstring for `init` indicates that "a strategy is dedicated to exactly one oneToken instance and must be re-initializable". It is not clear why it must be re-initializable, given that it is dedicated to a `oneToken` that cannot change.

LowUnlocked PragmaAcknowledged

Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.4.*`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked".