Links

Halborn April 2022 - Reserve and Controllers

Tokemak_Reserve_and_Controller_Smart_Contract_Security_Audit_Report 2.pdf
2MB
PDF
Individual auditors personal emails have been redacted in the above pdf (pdf also embedded below summary)
Summary:
  • 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.
  • 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.
  • 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.
  • 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.
  • 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.
  • 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: