CANISTER SMART CONTRACT AUDIT

Download Report
Download Report
Psychedelic DIP-20
September, 2023

Table of contents

Summary
Scope of Work
Workflow of the auditing process
Structure and organization of the findings
Manual Report

Summary

CAYA is pleased to present this audit report outlining our assessment of code, canister smart contracts, and other important audit insights and suggestions for management, developers, and users.
DIP20 is a fungible token standard for the Internet Computer. This standard allows for a common and familiar interface that not only provides a quick entry point for existing blockchain developers, but future interoperability options between the Internet Computer and Ethereum, through the process of sustaining the same shared interfaces. The scope of the audit includes the basic canister of the token standard, which carries out token transfer operations, mints, burns. It is also possible for the owner to set various token settings and manage the commission percentage. In addition, the canister defines different types of errors for each possible situation and provides for the recording of performed operations, that is, records of transactions are kept. It is worth noting that there are system functions that perform a number of actions in the case of a canister update. To support the logic of the token and the correctness of transactions, additional functions from auxiliary root files are also used.
CAYA is pleased to present this audit report outlining our assessment of code, canister smart contracts, and other important audit insights and suggestions for management, developers, and users.
Severity of the issue
Total found
Resolved
Unresolved
Critical
0 issues
0 issues
0 issues
High
0 issues
0 issues
0 issues
Medium
1 issue
0 issues
1 issue
Low
16 issues
0 issues
16 issues
Informational
8 issues
0 issues
8 issues
Total
25 issues
0 issues
25 issues
Evaluating the findings in this report, the CAYA auditors can state that the canisters are operational and secure. Under the given circumstances, we set the following risk level:
To set the codebase quality mark, our auditors are evaluating the initial commit given for the scope of the audit and the last commit with the fixes. This approach helps us adequately and sequentially evaluate the quality of the code. Code style, optimization of the canisters, the number of issues, and risk level of the issues are all taken into consideration. The CAYA team has developed a transparent evaluation codebase quality system presented below.
Severity of the issue
Issue severity
Total found
Resolved
Critical
1
10
High
0.8
7
Medium
0.5
5
Low
0.2
0.5
Informational
0
0.1
Please note that the points are deducted out of 100 for each and every issue on the list of findings (according to the current status of the issue). Issues marked as “not valid” are not subject to point deduction.
Score

Based on the given findings, risk level, performance, and code style, CAYA team can grant the following overall score:

Please be aware that this audit does not certify the definitive reliability and security level of the canister smart contract. This document describes all vulnerabilities, typos, performance issues, and security issues found by the CAYA audit team. If the code is still under development, we highly recommend running one more audit once the code is finalized.

Scope of Work

Psychedelic is a decentralized product studio building web3 products for an omni-chain world. Psychedelic has many developments in the field of ICP, among which it is worth highlighting the following: a bridge between EVM and ICP, an IC wallet, a NFT marketplace, etc. In addition to them, there is a developed basic DIP20 token on Rust and Motoko.
Within the scope of this audit, two independent auditors thoroughly investigated the given codebase and analyzed the overall security and performance of the canister smart contracts.
The audit was conducted on September, 2023. The outcome is disclosed in this document.
The scope of work for the given audit consists of the following canisters:
Cap.mo;
IC.mo;
Root.mo;
Router.mo;
Types.mo;
token.mo;
types.mo.
The source code was taken from the following source:
https://github.com/Psychedelic/DIP20/tree/main/motoko

Workflow of the auditing process

CAYA audit team uses the most sophisticated and contemporary methods and well-developed techniques to ensure canister smart contracts are free of vulnerabilities and security risks. The overall workflow consists of the following phases:

Phase 1: The research phase

Research
After the Audit kick-off, our security team conducts research on the contract’s logic and expected behavior of the audited canister smart contracts.
Documentation reading
CAYA auditors do a deep dive into your tech documentation with the aim of discovering all the behavior patterns of your codebase and analyzing the potential audit and testing scenarios.
The outcome
At this point, the CAYA auditors are ready to kick off the process. We set the auditing strategies and methods and are prepared to conduct the audit part.

Phase 2: Manual part of the audit

Manual check
During the manual phase of the audit, the CAYA team manually looks through the code in order to find any security issues, typos, or discrepancies with the logic of the contract. The initial commit as stated in the agreement is taken into consideration.
Candid UI check
In addition to the manual part and code review, having deployed canisters locally and using the Candid UI, we also test them in practice from the end user side and see how the canister reacts to various actions.
The outcome
An interim report with the list of issues.

Structure and organization of the findings

For simplicity in reviewing the findings in this report, CAYA auditors classify the findings in accordance with the severity level of the issues (from most critical to least critical).

All issues are marked as “Resolved” or “Unresolved”, depending on if they have been fixed by Psychedelic or not. The issues with “Not Valid” status are left on the list of findings but are not eligible for the score points deduction.

The CAYA team always provides a detailed description of the issues and recommendations on how to fix them.

Classification of found issues is graded according to 5 levels of severity described below:

Informational
Are classified as every point that increases onboarding time and code reading, as well as the issues which have no impact on the canister’s ability to operate.
Low
The issue doesn’t contain operational or security risks, but are more related to optimization of the codebase.
Medium
The issue slightly impacts the canister’s ability to operate by slightly hindering its intended behavior.
High
The issue significantly affects the ability of the canister smart contract to compile or operate. These are potential security or operational issues.
Critical
The issue affects the canister smart contract in such a way that funds may be lost or allocated incorrectly, or the issue could result in a significant loss.

Manual Report

The hardcoded value of fee

Medium
MM – 01
Unresolved
The token.mo canister has the genesis stable variable that saves zero value for the fee field. However the value of fee is set while creating and initializing canister smart contract. Besides, the canister has the accessor function to set the new fee. As a result, data saved in genesis are incorrect and will have an impact on the future.
Recommendation:
Consider the opportunity to set up the correct fee value.

Unspecified state visibility of types and variables

Low
ML – 01
Unresolved
In the Cap module and Token actor the global variables and types don't have clear visibility. Labeling the visibility explicitly is correct and makes it easier to catch incorrect assumptions about who can access the variable.
Recommendation:
Consider defining visibility for these objects.

Useless imports

Low
ML – 02
Unresolved
There are unused imports in the token.mo canister: Result and Text modules from the mo:base library.
Recommendation:
Consider removing useless imports to decrease the canister’s bytecode.

The variables that must be declared as constants

Low
ML – 03
Unresolved
The Token actor uses the stable variables whose values are immutable, but they are not declared as constants.
Recommendation:
Declare the following variables as constants: blackhole, decimals_, symbol_.

Local variable shadowing

Low
ML – 04
Unresolved
In the _chargeFee() function of token.mo canister appears shadowing because the canister also has a private stable variable ‘fee’.
Recommendation:
Rename the local variable in the function _chargeFee().

Optimization of error handling methods

Low
ML – 05
Unresolved
In the Cap.mo canister, 2 methods are used when processing a possible error:
1)
Function with a print template from the Prelude library -> Debug.trap(" Prelude.unreachable()")
2)
The function of printing a description of the error from the Debug library -> Debug.trap("Error while creating root bucket");
In fact, the Prelude library is not yet fully implemented according to the problem statement, but is only at the stage of future proposals for development. A good emphasis in programming is to stick to some one safe option.
Recommendation:
Use functions from the Debug library for correct error handling in the Cap.mo and types.mo or create custom error variants as done in the token.mo canister.

Optimizing the rootBucket value check

Low
ML – 06
Unresolved
In the Cap.mo canister, there is a separate function awaitForHandshake() to check the rootBucket value. Other functions that call this function have additional checks for the value of this variable. Accordingly, there is a redundancy of checks. In addition, the implementation of separate functions is very expensive, so the logic needs to be optimized.
For example, instead of
await awaitForHandshake();
let root = switch(rootBucket) {
    case(?r) { r };
    case(_) { Prelude.unreachable() };
};
you can do the following ->
if(rootBucket == null) performHandshake();
If this condition is not satisfied, it means that root is already present and the execution of the function will continue. Accordingly, in the performHandshake() function, no additional checks are needed.
Recommendation:
Optimize rootBucket validation and creation as per the example above. Such a solution will save money, time and optimize the code base.

Separate case without implementation

Low
ML – 07
Unresolved
The function performHandshake() in the Cap.mo has validation for the rootBucket variable. As result, only one case has described implementation. The case, when rootBucket is null, is without implementation.
However, if this function will be optimized and validation will be removed, this issue will be invalid.
Recommendation:
Consider the opportunity to fix this issue choosing the possible variant:
1)
Add a new implementation for case(_);
2)
Function awaitForHandshake() that calls this function already has checking for root, so remove this useless validation;
3)
Optimize all functions of this canister and this part of code will be unneeded.

Unnecessary create a new type

Low
ML – 08
Unresolved
A new global variable is created in the token.mo canister, which stores the value of Types.TxRecord. This variable is used only once, so it is pointless to allocate a memory area if this value is used only once. This is about an useless storage call.
Recommendation:
Remove unneeded additional declaration TxRecord type and use just Types.TxRecord where this is needed.

Useless storage call

Low
ML – 09
Unresolved
The token.mo canister initially has a balance definition for the owner. Data from global variables are specified as input parameters. You can optimize the use of cycles and memory by taking the constructor's input values. Example, instead of balances.put(owner_, totalSupply_); can be used balances.put(_owner, _totalSupply);
Recommendation:
Consider avoiding useless storage calls.

Lack checks on zero values

Low
ML – 10
Unresolved
The token.mo canister has transfers and approve functions. However, they lack checks for the possibility of a zero value of the input parameters, in particular for changing the value variable. In fact, such validations applied at the very beginning improve the efficiency of transactions and interactions between users and canisters.
Recommendation:
Consider adding require to provide input parameter validate.

Unused fields of the genesis stable variable

Low
ML – 11
Unresolved
The token.mo canister has the genesis stable variable that saves data of the TxRecord type. However, this variable is private and does not use in the canister, except the information about time of creating canister. As a result, data saved in genesis are unneeded.
Recommendation:
Consider adding all data to the query function that returns details about the token or remove unused fields.

Unused fields in the type TxReceipt

Low
ML – 12
Unresolved
The token.mo canister has a TxReceipt type defined, which stores a list of possible errors. However, not all options are used.
Recommendation:
In order to optimize the code and save memory, remove unnecessary fields or consider the possibilities of using them.

Unused declared types

Low
ML – 13
Unresolved
The token.mo canister has a declared private types. However, Operation and TransactionStatus types are not used.
Recommendation:
Remove unused types.

Optimize calculation of user’s balance

Low
ML – 14
Unresolved
The token.mo canister has a private function _transfer() that has the redundancy of creating additional variables. Example,
let from_balance = _balanceOf(from);
let from_balance_new : Nat = from_balance - value;
can be optimized as follows:
let from_balance = _balanceOf(from);
from_balance : Nat -= value;
In this way, operations are performed correctly and faster, without creating new variables.
Recommendation:
Consider optimizing calculations in the _transfer() function.

Useless condition in the _transfer() function

Low
ML – 15
Unresolved
In the private function _transfer() of token.mo canister exists a condition if (to_balance_new != 0) that is always true. The variable value should always be checked on zero values. As a result, to_balance will always be more than zero.
Recommendation:
If you haven't already added a check for zero values as recommended above, add it and remove the unnecessary condition, leaving only its body.

Useless condition in the transferFrom() function

Low
ML – 16
Unresolved
In the function transferFrom() of token.mo canister exists a condition if (allowed != 0) that is always true. The variable value should always be checked on zero values. In addition, this function has validation if allowance is sufficient for transfers value and fee. When this condition is false, then allowed will always be more than zero.
Besides, the result of the next line is need to receive before all conditions after transfers because this value is used in both cases later:
let allowance_from = Types.unwrap(allowances. get(from));
Recommendation:
If you haven't already added a check for zero values as recommended above, add it and remove the unnecessary condition, leaving only its body.

The unnecessary comments of code

Informational
MI – 01
Unresolved
Leaving commented-out code in source code is bad practice, as it takes up space, causes confusion, and leads to maintenance issues if it's not removed. The following practice occurs in the Cap.mo – 9, 104 lines and Router.mo – 1, 2 lines.
Recommendation:
Remove the commented code.

Lack of mo-doc annotations

Informational
MI – 02
Unresolved
Motoko's official documentation provides a number of rules regarding styling decisions for comments. Different options for commenting are used in the following canisters: Cap.mo, token.mo, types.mo.
Besides, the commenting methods /// and /** */ refer to generating the technical Motoko’s documentation. Doc comments should be used to provide explanations for functions, classes, types, modules, variables, and more.
Recommendation:
Add the mo-doc annotations for all objects in .mo-files following mo-doc rules and remove unneeded comments or rechange this to mo-doc style.

Naming conventions

Informational
MI – 03
Unresolved
Naming conventions in Motoko are important for creating clear, understandable and maintainable code. In the following canisters, global and local variables are incorrectly named: Cap.mo, token.mo.
Recommendation:
Use lowerCamelCase for constants and variant fields.

Order of Layout and Functions

Informational
MI – 04
Unresolved
The functions in the canister token.mo are not grouped according to their visibility and order. Functions should be grouped according to their visibility and ordered in the following way:
public;
private.
Besides, pay attention to logical dependencies in the following way:
accessors;
mutators;
processing.
As for the style of the Layout, it should be styled so that all types are defined next to each other, rather than scattered between functions.
Recommendation:
Layout and functions order should be grouped according to Motoko documentation.

Modules without names

Informational
MI – 05
Unresolved
Modules should be named according to their content and it is good practice to set the same file name as the module. No declared module has a name in any canister.
Recommendation:
Consider setting module names to match file names.

Rules of the style code

Informational
MI – 06
Unresolved
In all files, while writing the code, the styling rules regarding spacing are not followed. In addition, after writing each command or the end of a block, it is necessary to highlight it with ‘;’
Recommendation:
Consider using the rules while coding on Motoko from style guideline.

Unclear variables names

Informational
MI – 07
Unresolved
Basically, for easy reading and clear understanding, variable names should be given according to what they are used for and what data they store. The functions approve(), getHolders(), preupgrade() of the token.mo canister have declared variables with unclear names such as temp or v.
Recommendation:
Consider setting clear variables names in these functions.

Handling errors in accessors

Informational
MI – 08
Unresolved
Only the owner has access to the following functions for changing values: setName(), setLogo(), setFee(), setFeeTo(), setOwner(). Checking whether the caller is the owner is done using the assert statement, which works in such a way that if it returns a false, the program simply stops working. However, the canister also defines custom types for owner deauthorization. To understand the cause of the error, use the condition with the corresponding error return. Example:
if(msg.caller != owner_) {
   return #Err(#Unauthorized); // or Debug.trap();
};
Recommendation:
Consider using the understandable condition with clear reason of the possible error.
We are delighted to have a chance to work with the Psychedelic team and contribute to your company's success by reviewing and certifying the security of your canister smart contracts.
The statements made in this document should be interpreted neither as investment or legal advice, nor should its authors be held accountable for decisions made based on this document.
Our CEO
If you are looking for not execution of your scope only but to have a long-term visionary partner you are in the right place!
Book a Call
@ian_arden
ian@caya.cc
Contact US
Contact us today to get a free consultation on security auditing, management, and blockchain development.
Submit
Thank you! Your submission has been received!
Oops! Something went wrong while submitting the form.