Halborn April 2022 - Reserve and Controllers
Summary:
(HAL-01) UNCHECKED TOKEN ADDRESS
Recommendation: It is recommended to ensure that the addresses provided are legitimate. To prevent attacks like this from happening, make sure the provided address is registered in the Address Registry, or even consider hardcoding the WETH contract address.
Risk Level: Medium
Status: SOLVED - 07/04/2022
Additional Notes: The Tokemak Team solved this issue by removing the previous weth address declaration and configuring the query from AddressRegistry contract instead.
(HAL-02) EXPERIMENTAL FEATURES ENABLED
Recommendation: When possible, do not use experimental features in the final live deployment. Validate and check that all the above conditions are true for integers and arrays (i.e., all using uint256).
Risk Level: Low
Status: RISK ACCEPTED
Additional Notes: Removed from one of the controllers as it was not being used. Verified to not meet the criteria that would manifest an encoding bug in the other.
(HAL-03) MULTIPLE ROLE-BASED ACCESS CONTROL MECHANISMS IMPLEMENTED
Recommendation: It is recommended to follow a uniform pattern when developing functionalities across different contracts. This is considered a good 19 FINDINGS & TECH DETAILS practice, and having functionalities like RBAC implemented only once will significantly ease any further code modification or role management operation that might be necessary.
Risk Level: Informational
Status: ACKNOWLEDGED
Additional Notes: The Tokemak Team acknowledged this issue and confirmed that Controllers are deployed with the accessControl variable set to Manager address.
(HAL-04) MULTIPLE INITIALIZATION CRITERIA
Recommendation: It is recommended to follow a uniform pattern when defining deploying and initialization criteria along smart contracts. This is considered a good practice and will improve code readability and usability.
Risk Level: Informational
Status: SOLVED - 07/04/2022
Additional Notes: The Tokemak Team solved this issue by standardizing the initialization criteria, adding the missing constructor to Pool.sol and EthPool.sol contracts.
(HAL-05) UNDERLYING TOKEN NOT ENFORCED
Recommendation: Apparently, this smart contract is designed to work with wETH as an underlying token. In that case, it is recommended to check that pool. underlyer() is wETH actual direction to prevent deploying a nonfunctional smart contract.
Risk Level: Informational
Status: SOLVED - 07/04/2022
Additional Notes: The Tokemak Team solved this issue by using the wETH token address defined in AdressRegistry.sol instead of setting it again when initializing the contract.
(HAL-06) CONFUSING VARIABLE NAMING
Recommendation: It is considered a good practice to follow a uniform naming criteria, avoiding the usage of similar variable names.
Risk Level: Informational
Status: SOLVED - 07/04/2022
Additional Notes: The Tokemak Team solved this issue by maintaining the name lpToken, and changing lptoken to poolLpToken in the ConvexController.sol contract.
PDF Embed:
Last updated