JPEG'd contest - Cr4ckM3's results

Better noContract

noContract modifier is used in farming/LPFarming.sol#L85 to prevent non-whitelisted contracts to interact with the LP farm. But isContract check can be bypassed with a contract executing constructor code.

require(msg.sender == tx.origin) may be a better choice.

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L459

Vulnerability details

Impact

According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0.

Proof of Concept

NFTVault.sol#L459

It's same as a medium risk issue in other C4 contest

Tools Used

N/A

Recommended Mitigation Steps

Use the new latestRoundData function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is uncompleted, for example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = oracle.latestRoundData();
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");

Redundant onlyOwner

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L363

As we always revert in renounceOwnership(), onlyOwner check can be removed.