QA Report

[code423n4]

1. User facing functions miss emergency lever

Impact

If there be any emergency with system contracts or outside infrastructure, there is no way to temporary stop the operations.

Proof of Concept

Most core user-facing contracts do not have pausing functionality for new operation initiation.

For example, NFTVault’s user facing functions cannot be temporary paused:

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

LPFarming also can benefit from having pausing functionality:

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

Recommended Mitigation Steps

Consider making all new actions linked user facing functions pausable.

For example, by using OpenZeppelin's approach:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol

2. Floating pragma is used in most contracts

As different compiler versions have critical behavior specifics if the contracts get accidentally deployed using another compiler version compared to the one they were tested with, various types of undesired behavior can be introduced

Proof of Concept

pragma solidity ^0.8.0 is used in most contracts, for example:

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

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

Recommended Mitigation Steps

Consider fixing the version to 0.8.x across all the codebase, for example set x to 10

3. YVaultLPFarming's claim() doesn't have partial claim option

Impact

If there will be not enough reward funds the biggest claimers will have their share of rewards frozen.

I.e. if there be not enough jpeg in the strategy for any reason the claim will fail as it goes for the full amount each time.

Proof of Concept

Only full claim is now allowed in yVaultLPFarming:

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/yVaultLPFarming.sol#L132-L134

Recommended Mitigation Steps

Introduce an argument to perform the partial claim.

4. StrategyPUSDConvex withdraw withdrawAndUnwrap

Impact

User facing YVault withdraw will fail with low level lack of funds message if StrategyPUSDConvex fails to obtain enough funds as this isn't controlled.

For example, in case of underlying strategy having any liquidity issues the withdraw will fail without proper error even when there are enough funds due for withdrawal.

Proof of Concept

StrategyPUSDConvex's withdraw assumes that withdrawAndUnwrap will always obtain the _amount - balance needed:

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

yVault's withdraw also doesn't check how much was retrieved:

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/yVault.sol#L177-L182

Recommended Mitigation Steps

Consider checking for the necessary amount obtained by withdrawAndUnwrap in StrategyPUSDConvex's withdraw and failing with clear error if there is not enough funds available at the moment to fulfil the request