search

code-423n4 / 2022-05-alchemix-findings

5 stars 2 forks source link

QA Report #211

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago
  1. TransferOwnership should be two step process

Lack of two-step procedure for critical operations leaves them error-prone if the address is incorrect, the new address will take on the functionality of the new role immediately

for Ex : -Alice deploys a new version of the whitehack group address. When she invokes the whitehack group address setter to replace the address, she accidentally enters the wrong address. The new address now has access to the role immediately and is too late to revert

Affected functions: gALCX.transferOwnership()

Tools Used

Manual review

Recommended Mitigation Steps

use a two-step procedure for all non-recoverable critical operations to prevent irrecoverable mistakes.

  1. Missing zero address check The following functions have missing zero address check for the corresponding parameter

EthAssetManager.setOperator() EthAssetManager.setRewardReceiver() EthAssetManager.setTransmuterBuffer() AlchemicTokenV2Base.mint() - recipient param AlchemicTokenV1.mint() - recipient param

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding zero address check

  1. Unsafe approve in AutoleverageBase.sol The functions below made an approve call without checking the boolean result. ERC20 standard specify that the token can return false if the approve was not made, so it's mandatory to check the result of approve methods.

Functions affected: AutoleverageBase.approve() AutoleverageCurveMetapool._curveSwap() WETHGateWay.refreshAllowance()

Tools Used

Manual review

Recommended Mitigation Steps

Use safe approve or check the boolean result