AlphaHomoraV2 security record

`contracts/HomoraBank.sol`: function `borrow` should check that `collateralValue > borrowValue` right after transferring the borrowed token out of the contract.

`contracts/HomoraBank.sol`: function `takeCollateral` should check `require(collateralValue > borrowValue, 'position still healthy');` right after transferring the collateral out of the contract.

`L355` in `contracts/HomoraBank.sol`, should be `amount.mul(totalShare).div(totalDebt)` instead of `amount.mul(totalDebt).div(totalShare)`.

Uniswap oracle (https://uniswap.org/docs/v2/core-concepts/oracles/) aggregates prices from all the blocks weighted by block time into cumulative prices whereas Keep3r oracle takes 30-minute samples of Uniswap cumulative prices. The price calculations (i.e., function `price0TWAP` and `price1TWAP`) in `contracts/oracle/BaseKP3ROracle.sol` calculates the sample provided by Keep3r with a last-minute (`now`) spot price from Uniswap. This is fine as long as the accumulation weight is okay, but under heavy congestion and delays the weight (i.e., `timeElapsed` in function `currentPx0Cumu` and `currentPx1Cumu`) can be too big. And together with the (`IUniswapV2Pair(pair).getReserves()`) in `BaseKP3ROracle`, this platform could be attacked by flash loans. Several recent attacks were documented in https://samczsun.com/so-you-want-to-use-a-price-oracle/. **2020-12-18 update**: Alpha team stated that only LP tokens will be used as collateral, so the price data eventually used will not be influenced by a flash-loan attack.

In `contracts/HomoraBank.sol`, the function `liquidate` never clears up the collateral value in the liquidated position. With the function `takeCollateral` in its form currently, this could cause fund loss. **2020-12-18 update**: Alpha team enables the function to subtract the collateral value with the bounty provided after a position being liquidated. Quantstamp would like to further suggest to consider to directly assign zero to the collateral value when bounty is larger than the collateral value of a position, in order to avoid unexpected failure of liquidation due to the line `pos.collateralSize = pos.collateralSize.sub(bounty);`. **2021-01-13 update**: this one is fixed in <https://github.com/AlphaFinanceLab/homora-v2/commit/233de49b1e4ec0158861b611de153bf95dcf4aab>.

`contracts/oracle/ProxyOracle.sol`: multiplication is performed after a truncation division in a series of integer calculations. This leads to miscalculation and will lead to a financial loss over time or cause unexpected results. For instance, `contracts/oracle/ProxyOracle.sol`: `L77`, `L89`-`L90`, and `L96`-`L97`. In addition, taking function `asETHCollateral()` as an example: 1. getETHPx() = 100.5 and amount = 0.05. 1. Before truncation = 50.25 1. After truncation = 50.00 1. collateralFactor = 10,000 1. Final value = 50.000 and value lost close to 0.5% from original 50.25 **2020-12-18 update**: Alpha team stated that it is intended. The deviation is bounded by `borrowFactor`/10000 (in wei). The maximum value for `borrowFactor` value will be in the order of 10^6, bounding the error by ~100 wei, which will be less than a block’s interest accrued.

Integer overflow/underflow occur when an integer hits its bit-size limit. Every integer has a set range; when that range is passed, the value loops back around. A clock is a good analogy: at 11:59, the minute hand goes to 0, not 60, because 59 is the largest possible minute. Integer overflow and underflow may cause many unexpected kinds of behavior and was the core reason for the `batchOverflow` attack. Here's an example with `uint8` variables, meaning unsigned integers with a range of `0..255`. ``` function under_over_flow() public { uint8 num_players = 0; num_players = num_players - 1; // 0 - 1 now equals 255! if (num_players == 255) { emit LogUnderflow(); // underflow occurred } uint8 jackpot = 255; jackpot = jackpot + 1; // 255 + 1 now equals 0! if (jackpot == 0) { emit LogOverflow(); // overflow occurred } } ``` `SafeMath` was not enforced for arithmetic operations. This would greatly increase the chances of this dapp having underflow/overflow issues and lead to unexpected results. For instance, in `BaseKP3ROracle.sol`, `L28`, `L32`, `L35`, `L44`, `L48`, `L51`, `L61`, `L62`, and `L73`. **2020-12-18 update**: Alpha team considered there is no need for using library `SafeMath` for the operations pointed out, and the overflow is intended by design. In addition, QuantStamp would also like to point out that `L57` could conceivably overflow in the future (year 2038 problem). **2021-01-13 update**: It is confirmed that for `L55` the `uint32` overflow is an expected behavior. See https://uniswap.org/whitepaper.pdf section 2.2.1 for more details.

`contracts/HomoraBank.sol`: function `setCToken` should `safeApprove` the newly added `cToken`.

`contracts/HomoraBank.sol`: function `initialize` should check if `IOracle _oracle` is not `0x0`.

Potentially high gas usage in `getBorrowETHValue` as the number of banks (i.e., supported tokens) increases. **2020-12-18 update**: Alpha team fixes this issue by using a bitmap to iterate over banks debt.