A technical audit of Phuture finance === (Awarded: 200 USD) ## Low-Medium severity issues ### Uninitialised state variables [`IndexLayout.factory`](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLayout.sol#L13) is uninitialised when being used in [`_chargeAUMfees()`](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/PhutureIndex.sol#L65). A possible exploit here would be to use `factory.transfer()` pre-emptively thus having a `0x00` address. ### Multiplication on Division In the core contracts, there are two instances where multiplication is done on a result of division. This might result in loss of precision in precision-sensitive DeFi contracts. ``` value = (oracle.convertToIndex(minAmountInBase, decimals()) * totalSupply()) / oracle.convertToIndex(lastAssetBalanceInBase, decimals()); ``` Found [here](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L78) (line 77 and 85) and [here](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/libraries/AUMCalculationLibrary.sol#L55). Please note, `FullMath.sol` library also does this, but it seems to be a well audited third party library, thus not mentioning here. <b>Why should you care:</b> Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. ### Dependency-based Vulnerabilities Phuture core contracts use UniswapV2OracleLibrary [here](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/UniswapV2PriceOracle.sol#L15) which might result in violation of [SWC-116](https://swcregistry.io/docs/SWC-116) - while not severe, it is usually suggested to not use timestamp from Blocks. ### Strict Operators Should not be an issue here, but `IndexLogic` has a strict unequality [here](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L134) - usually not recommended due to possible workaround exploits using this strict condition. Just wanted to point out. ### Zero Checks [`BaseIndex.mint(address)._recipient`](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/BaseIndex.sol#L43) doesn't have a zero address check which can potentially be used to drain balance via indirect exploits. ### Re-entrancies ``` function reweight() external override onlyRole(ORDERER_ROLE) { (bool success, bytes memory data) = IIndexFactory(factory).reweightingLogic().delegatecall( abi.encodeWithSelector(ITopNMarketCapIndexReweightingLogic.reweight.selector, category, snapshot, topN) ); if (!success) { if (data.length == 0) { revert("TopNMarketCapIndex: REWEIGH_FAILED"); } else { assembly { revert(add(32, data), mload(data)) } } } snapshot = abi.decode(data, (uint)); } ``` Found [here](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/TopNMarketCapIndex.sol#L68). There is a medium severity re-entrancy vulnerability here. While the role is limited, but a wrong chain of executions can allow for re-entrancy via `snapshot = abi.decode(data, (uint));` [here](https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/TopNMarketCapIndex.sol#L81). There are other benign re-entrancies that do not need reporting or concern as far as I can tell, but here are a few examples anyway. - `BaseIndex.constructor(address)` (contracts/BaseIndex.sol#33-40) - `ManagedIndexReweightingLogic.reweight(address[],uint8[])` (contracts/ManagedIndexReweightingLogic.sol#28-105) ### Favoring Pull over Push I couldn't find a direct violation of this standard, but I did notice a lot of calls inside loops. This is often neccessary in complex DeFi protocols but causes low efficiency.