The developers of Toucan Protocol contracted byterocket to conduct a smart contract audit of their Nature Carbon Tonne (NCT) smart contract. Toucan is building a protocol that “brings programmable carbon to Web3, unlocking its potential for a regenerative economy.”
The team of byterocket reviewed and audited the above smart contracts in the course of this audit. We started on the 21st of January and finished on the 24th of February 2022.
The audit included the following services:
byterocket gained access to the code via a private GitHub repository. We based the audit on the main branch of the repository, deployed on January 30th, 2022 (commit hash f79bfcf34cfe0c742be8d48a98c6aea289256d7a). The updated version was provided to us on March 4th, 2022 via two new commits to the private repository, addressing our findings. The last and most recent commit hash that we audited is 39e49715579a47921e8940f5ca0aaff05dfd9cef.
We conducted a manual multi-pass code review of the smart contracts mentioned in section (1). Three different people went through the smart contract independently and compared their results in multiple concluding discussions.
The manual review and analysis were additionally supported by multiple automated reviewing tools, like Slither, GasGauge, Manticore, and different fuzzing tools.
We are categorizing our findings into four different levels of severity:
On the code level, we found 6 bugs or flaws, with 6 of them being fixed in a subsequent update. Prior to this, there have been 6 non-critical findings.
The contracts are written according to the latest standard used within the Ethereum community and the Solidity community’s best practices, with some exceptions listed below. The naming of variables is very logical and understandable, which results in the contract being useful to understand. The code is well documented. The developers provided us with a test suite as well as deployment scripts.
Location: NCT.sol - Line 328 - 331
Description:
The documentation of this function has a slight inaccuracy. It can only be called by the manager role, while the documentation states that only owners may call it.
These are the affected lines of code:
Recommendation:
Consider updating the documentation to match the implementation.
Update on the 29th of March:
The corresponding documentation has been updated.
Location: NCT.sol - Line 75 & BaseCarbonTonne.sol - Line 82
Description:
The initialize() function is using the _setupRole() function to set up roles. This function has been deprecated, as referenced here. The new function is called _grantRole().
These are the affected lines of code:
Recommendation:
While _setupRole() internally just calls _grantRole(), it just makes sense to directly call _grantRole().
Update on the 29th of March:
This has been changed; the new function is now being used.
Location: Throughout NCT.sol & BaseCarbonTonne.sol
Description:
There are some functions (as described below) that change important state variables like supply caps or contract registries, which do not emit an event. Usually, it is the best practice to emit an event here.
These are the affected functions:
Recommendation:
Consider adding a corresponding event to each of these functions.
Update on the 29th of March:
For each of our five recommended functions that didn’t emit an event previously, there is now an event being emitted.
Location: NCT.sol - Line 318 - 326
Description:
The setMinimumVintageStartTime() function sets a quite important timestamp without checking the input. If this timestamp would be set in the future, deposits would be disabled for ToucanContracts that are not whitelisted. It is usually the best practice to validate important inputs.
These are the affected lines of code:
Recommendation:
Consider adding a require statement that ensures, that invalid (e.g. future) timestamps can not be set.
Update on the 29th of March:
The team has acknowledged this finding. Due to a well-defined process from their side when calling these functions, they can be ensured, that this value will not be set incorrectly. The contract size tradeoff is a good reason to not add this check here.
Location: NCT.sol - Line 318 - 326
Description:
The checkEligible function returns whether a certain token is eligible to be deposited. This technically sounds like a function that some users might benefit from having publicly available before they deposit a certain token. It did not seem very clear why this function has been implemented as internal.
These are the affected lines of code:
Recommendation:
Consider setting functions to public that might provide a benefit for some users if the function does not have to be internal.
Update on the 29th of March:
The function visibility has been set to public.
Location: NCT.sol - Line 344 - 367
Description:
Due to the positioning of the safeTransferFrom call in the deposit function, it is technically possible to execute a reentrancy attack for certain malicious tokens that are implemented in a certain way.
These are the affected lines of code:
Recommendation:
The chances of this happening are minimal since this is more of a theoretical threat. We mainly raised this issue to emphasize the fact that the team has to be very diligent when it comes to adding other tokens.
Update on the 29th of March:
The team acknowledged our finding and ensured that there is a diligent process in place that verifies any potential tokens that are being used.
Part of our audits are also analyses of the protocol and its logic. The byterocket team went through the implementation and documentation of the implemented protocol.
The repository itself contained tests and documentation. We found the provided unit tests that are coming with the repository execute without any issues and cover the most important parts of the protocol. There are some unit tests, where the output is only being printed and not verified through an expect or assert statement.
According to our analysis, the protocol and logic are working as intended, given that the findings listed in section (2) have been fixed.
We were not able to discover any additional problems in the protocol implemented in the smart contract.
During our code review (which was done manually and automated), we found 8 bugs or flaws, with 8 of them being fixed in a subsequent update. Prior to this, there have been 6 non-critical and 2 low severity findings. Our automated systems and review tools did not find any additional ones.
The protocol review and analysis did neither uncover any game-theoretical nature problems nor any other functions prone to abuse.
In general, there are some improvements that can be made, but we are very happy with the overall quality of the code and its documentation. The developers have been very responsive and were able to answer any questions that we had.
As of the date of publication, the information provided in this report reflects the presently held understanding of the auditor’s knowledge of security patterns as they relate to the client’s contract(s), assuming that blockchain technologies, in particular, will continue to undergo frequent and ongoing development and therefore introduce unknown technical risks and flaws. The scope of the audit presented here is limited to the issues identified in the preliminary section and discussed in more detail in subsequent sections. The audit report does not address or provide opinions on any security aspects of the Solidity compiler, the tools used in the development of the contracts or the blockchain technologies themselves, or any issues not specifically addressed in this audit report.
The audit report makes no statements or warranties about the utility of the code, safety of the code, suitability of the business model, investment advice, endorsement of the platform or its products, the legal framework for the business model, or any other statements about the suitability of the contracts for a particular purpose, or their bug-free status.
To the full extent permissible by applicable law, the auditors disclaim all warranties, express or implied. The information in this report is provided “as is” without warranty, representation, or guarantee of any kind, including the accuracy of the information provided. The auditors hereby disclaim, and each client or user of this audit report hereby waives, releases and holds all auditors harmless from, any and all liability, damage, expense, or harm (actual, threatened, or claimed) from such use.
We store our public audit reports on IPFS; a peer-to-peer network called the "Inter Planetary File System". This allows us to store our reports in a distributed network instead of just a single server, so even if our website is down, every report is still available.
The IPFS Hash, a unique identifier of the report, is signed on-chain by both the client and us to prove that both sides have approved this audit report. This signing mechanism allows users to verify that neither side has faked or tampered with the audit.
