Open code423n4 opened 2 years ago
CvxCrvRewardsLocker
ConvexStrategyBase.sol
If any constant address in CvxCrvRewardsLocker changes, they will not be able to be updated.
Also valid for(list not exhaustive): -CTokenRegistry -ConvexStrategyBase -StrategySwaper
CvxCrvRewardsLocker.setTreasury; L147-L154
Accidentally setting treasury to address zero may lead to losing all funds when withdrawing.
treasury
TopUpActionFeeHandler.actionContract#L55
If actionContract is accidentally set to zero the payFees function would become inaccessible because there is no other way to set actionContract.
actionContract
payFees
safeApprove
safeApprove from OpenZeppelin has been deprecated. Instead it is recommended to use increase/decrease allowance.
Please refer to SafeERC20.sol#L38
CvxCrvRewardsLocker.withdraw#L156-L164 CvxCrvRewardsLocker.withdraw#L217-L225
Suggest changing withdraw#L156-L164 to fullWithdraw to avoid confusion and improve readability.
CompoundHandler
CompoundHandler#L46 Change tup-up to top-up.
StakerVault.unstakeFor
StakerVault.sol#L360
Hook in ERC777 reenters unstakeFor and bypasses _allowances update which is only calculated after safeTransfer.
unstakeFor
_allowances
safeTransfer
Beaware that if LpToken ever uses the ERC777 standard this could be an attack vector.
I consider this report to be of particularly high quality
LOW
L-01: If any constant address in
CvxCrvRewardsLocker
changes, they will not be able to be updated.ConvexStrategyBase.sol
If any constant address in
CvxCrvRewardsLocker
changes, they will not be able to be updated.Also valid for(list not exhaustive): -CTokenRegistry -ConvexStrategyBase -StrategySwaper
L-02: No zero address checks
CvxCrvRewardsLocker.setTreasury; L147-L154
Accidentally setting
treasury
to address zero may lead to losing all funds when withdrawing.TopUpActionFeeHandler.actionContract#L55
If
actionContract
is accidentally set to zero thepayFees
function would become inaccessible because there is no other way to setactionContract
.L-03:
safeApprove
from OpenZeppelin has been deprecatedsafeApprove
from OpenZeppelin has been deprecated. Instead it is recommended to use increase/decrease allowance.Please refer to SafeERC20.sol#L38
NON-CRITICAL
N-01: Confusing function naming
CvxCrvRewardsLocker.withdraw#L156-L164 CvxCrvRewardsLocker.withdraw#L217-L225
Suggest changing withdraw#L156-L164 to fullWithdraw to avoid confusion and improve readability.
No-02: Minor typo in
CompoundHandler
CompoundHandler#L46 Change tup-up to top-up.
N-04: ERC777 Reentrancy in
StakerVault.unstakeFor
could bypass allowance.StakerVault.sol#L360
Hook in ERC777 reenters
unstakeFor
and bypasses_allowances
update which is only calculated aftersafeTransfer
.Beaware that if LpToken ever uses the ERC777 standard this could be an attack vector.