It is recommended to implement an acceptOwnershipTransfer() function to reduce the risk of transferring ownership to the wrong account or zero address. The pattern would include a transferOwnership() function to set the pending new owner, and the acceptOwnershipTransfer() function would have to be called by the pending owner for the transfer to take effect.
All contracts contain a floating pragma. It is recommended to deploy all contracts with a single, specific compiler version to reduce the risk of compiler-specific bugs and contracts deployed with different versions. In the case of the forked contacts, I recommend deploying with the exact version that the current live versions were deployed with.
Some tokens like USDT do not allow you to approve from a non-zero amount to another non-zero amount. Therefore, the following code will need to reset the approval to 0 prior to approving another non-zero amount.
Issue 1 (Low) - No ownership transfer pattern
It is recommended to implement an acceptOwnershipTransfer() function to reduce the risk of transferring ownership to the wrong account or zero address. The pattern would include a transferOwnership() function to set the pending new owner, and the acceptOwnershipTransfer() function would have to be called by the pending owner for the transfer to take effect.
Link to code
https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/MasterPriceOracle.sol#L81-L85
Issue 2 (Low) - Missing Zero Address Checks
Several files do not check that the addresses set to variables are not address(0).
https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/MasterPriceOracle.sol#L52-L53 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/UniswapTwapOracle.sol#L93-L96 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/UniV3LpVault.sol#L65 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/Comptroller.sol#L82 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/CErc20.sol#L42
Issue 3 (Low) - Floating pragma
All contracts contain a floating pragma. It is recommended to deploy all contracts with a single, specific compiler version to reduce the risk of compiler-specific bugs and contracts deployed with different versions. In the case of the forked contacts, I recommend deploying with the exact version that the current live versions were deployed with.
https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/CToken.sol#L1 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/CErc20.sol#L1 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/Comptroller.sol#L1 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/UniV3LpVault.sol#L1 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/FlashLoan.sol#L1 https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/UniswapTwapOracle.sol#L1
Issue 4 (Low) - Need to approve 0 for some tokens
Some tokens like USDT do not allow you to approve from a non-zero amount to another non-zero amount. Therefore, the following code will need to reset the approval to 0 prior to approving another non-zero amount.
https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/FlashLoan.sol#L48-L58