No address(0) or Zero value check at the constructors/initiliazers of Authorization.sol , Vault.sol, ETHVault.sol, ERC20Vault.sol, StakerVault.sol,TopUpAction.sol and many other contracts.
At Vault.sol#233, set() method is inherited inside executeNewStrategy(). It requires 3 params but only 2 params used. It will fail and lock the funds inside if there are any funds left due to locking deadlines.
At Vault.sol#L323, logic should be;
if (totalDebt > debtLimitAllocated)
instead of;
if (totalDebt >= debtLimitAllocated)
transfer method is used severalplacesinside the codebase. Since transfer method uses 2300 gas stipend which is not adjustable,it may likely to get broken in future in case of hardforks causing gas price changes or when calling a contract's fallback function.
Reference Link -1, Reference Link -2
There is a function overloading starting with AddressProvider.sol' allActions() method. allActions() returns _actions.toArray() for where it calls toArray(EnumerableSet.Bytes32Set storage values) of EnumerableExtensions.sol. However, this function affects the state changes and state changes should not be based upon it as stated here due to function has unbounded cost.
At TopUpAction.sol#L943 , TopUpActipnFeeHandler.sol#L232 , CTokenRegistry.sol#L67, Using abi.encodePacked() with multiple variable length arguments can, in certain situations, lead to a hash collision. Instead abi.encode() can be used. Reference Link
Team can consider to remove TODO's at TopUpAction.sol#L713 , ConvexStrategyBase.sol#L4-L5
QA (LOW / NON-CRITICAL)
No
address(0)or Zero value check at the constructors/initiliazers ofAuthorization.sol,Vault.sol,ETHVault.sol,ERC20Vault.sol,StakerVault.sol,TopUpAction.soland many other contracts.At
Vault.sol#233,set()method is inherited insideexecuteNewStrategy(). It requires 3 params but only 2 params used. It will fail and lock the funds inside if there are any funds left due to locking deadlines.At
Vault.sol#L323, logic should be;instead of;
transfermethod is used severalplacesinside the codebase. Sincetransfermethod uses 2300 gas stipend which is not adjustable,it may likely to get broken in future in case of hardforks causing gas price changes or when calling a contract's fallback function. Reference Link -1, Reference Link -2There is a function overloading starting with
AddressProvider.sol'allActions()method.allActions()returns_actions.toArray()for where it callstoArray(EnumerableSet.Bytes32Set storage values)ofEnumerableExtensions.sol. However, this function affects the state changes and state changes should not be based upon it as stated here due to function has unbounded cost.At
TopUpAction.sol#L943,TopUpActipnFeeHandler.sol#L232,CTokenRegistry.sol#L67, Usingabi.encodePacked()with multiple variable length arguments can, in certain situations, lead to a hash collision. Insteadabi.encode()can be used. Reference LinkTeam can consider to remove TODO's at
TopUpAction.sol#L713,ConvexStrategyBase.sol#L4-L5Usage of deprecated
safeApprove. LinkTypo at
ChainlinkOracleProvider.sol#L37, Correct wording should bestaleinsted ofstakewhich is confusing