Phuture Finance contest - abhinavmir's results

Non-critical risks on Phuture

Uninitialised state variables

IndexLayout.factory is uninitialised when being used in _chargeAUMfees().

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 (line 77 and 85) and here. 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 which might result in violation of 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 - usually not recommended due to possible workaround exploits using this strict condition. Just wanted to point out.

Zero Checks

BaseIndex.mint(address)._recipient 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. 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.

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.