JPEG'd contest - JMukesh's results

Lines of code

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

Vulnerability details

Impact

latestanswer() , this method does not error if no answer has been reached, it will simply return 0, since we have checks in function require(answer > 0, "invalid_oracle_answer"); we may not get the latest value of current price which can affect the functionality depending on this.

Proof of Concept

`function _normalizeAggregatorAnswer(IAggregatorV3Interface aggregator) internal view returns (uint256) { int256 answer = aggregator.latestAnswer(); uint8 decimals = aggregator.decimals();

    require(answer > 0, "invalid_oracle_answer");
    //converts the answer to have 18 decimals
    return
        decimals > 18
            ? uint256(answer) / 10**(decimals - 18)
            : uint256(answer) * 10**(18 - decimals);
}

`

https://etherscan.io/address/0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419#code#L142

Tools Used

manual review

Recommended Mitigation Steps

use latestRoundData() to get the price

1. specification not according to the comments

impact

In NftVault.liquidate(uint) , comment says Positions can only be liquidated once their debt amount exceeds the minimum liquidation debt to collateral value rate

but in the function position get liquidated even when it is equal to min liquidation debt

require( debtAmount >= _getLiquidationLimit(_nftIndex), "position_not_liquidatable" );

proof

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

Recommended Mitigation Steps

change >= to > require( debtAmount > _getLiquidationLimit(_nftIndex), "position_not_liquidatable" );

2. lack of access modifier in Intialize()

impact

Due to lack of access modifier in intialize() anyone can call the initialize the function with their implementation

proof

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

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L66

3.Not all the ERC20 tokens have openzeppelin implementation

impact

since all the ERC20 token does not have openzeppelin implementation , due to some token may not be compatible with it. In yVault.sol constructor iniatialize the token with token = ERC20(_token);, insteal of ERC20 it should be IERC20

Proof

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/yVault.sol#L53

4.No checks for duplicate value in Array

impact

Due to no checks for the duplicate values in array same categories will be added to the nftTypeValueETH[initializer.hash]

proof

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

1. Caching the length in for loops

impact

the solidity compiler will always read the length of the array during each iteration. That is,

1. if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929) for each iteration except for the first),

2. if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first),

3. if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)

This extra costs can be avoided by caching the array length (in stack): uint length = arr.length; for (uint i = 0; i < length; i++) { // do something that doesn't change arr.length }

proof

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

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L145

2 .use of != for comparison instead of > 0

impact

!= 0 costs less gas compared to > 0 for in require statements

proof

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