Phuture Finance contest - fatima_naz's results

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/ChainlinkPriceOracle.sol#L83-L86

Vulnerability details

Impact

Price can be stale which can lead to wrong assetPerBaseInUQ return value

Proof of Concept

Oracle data feed is insufficiently validated. There is no check for stale price and round completeness.

Tools Used

Manual review, similar issue was found in yield contest - https://github.com/code-423n4/2022-01-yield-findings/issues/136

Recommended Mitigation Steps

Validate data feed

    (uint80 roundID, int256 basePrice, , uint256 timestamp, uint80 answeredInRound) = 
    baseAggregator.latestRoundData();
    require(basePrice > 0, "ChainLink: Base price <= 0");
    require(answeredInRound >= roundID, "ChainLink: Stale price");
    require(timestamp > 0, "ChainLink: Round not complete");
    (roundID, int256 quotePrice, , timestamp, answeredInRound) = assetInfo.aggregator.latestRoundData();
    require(usdcPrice > 0, "ChainLink: Quote price <= 0");
    require(answeredInRound >= roundID, "ChainLink: Stale price");
    require(timestamp > 0, "ChainLink: round not complete");

1)- __gap is defined as storage variable but never used, - In vToken.sol https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/vToken.sol#L222

2)- As mentioned in github description of ManageIndexReweightingLogic.sol - This index reweighs manually by it's admin. shouldn't we put some modifier for reweight function! it is defined as external but as the users of protocol need not to call this function. adding some modifier could be a good practice.

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/ManagedIndexReweightingLogic.sol#L28

similary here for TopNMarketCapReweightingLogic.sol

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/TopNMarketCapReweightingLogic.sol#L30-L34

1)- initialization of uint in for loop, default value of uint is zero - UniswapV2PathPriceOracle.sol https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/UniswapV2PathPriceOracle.sol#L49

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/UniswapV2PathPriceOracle.sol#L34

2)- use (!=0) instead of >0 ( gas efficient). uint can't be negative - https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L76

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L86

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L98

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L114

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/IndexLogic.sol#L141

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/ManagedIndexReweightingLogic.sol#L56

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/ManagedIndexReweightingLogic.sol#L61

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/vToken.sol#L160

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/TopNMarketCapReweightingLogic.sol#L58

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/TopNMarketCapReweightingLogic.sol#L79

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/TopNMarketCapReweightingLogic.sol#L106

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/PhutureIndex.sol#L56

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/PhutureIndex.sol#L64

3)- use require instead of assert. require will revert and return the gas at failure. https://github.com/code-423n4/2022-04-phuture/blob/47cd226c80842585542599a3b56cc2a26b519d8a/contracts/IndexLogic.sol#L72