Prepared by:
HALBORN
Last Updated 04/26/2024
Date of Engagement by: February 9th, 2022 - February 18th, 2022
75% of all REPORTED Findings have been addressed
All findings
4
Critical
0
High
0
Medium
0
Low
1
Informational
3
Astroport.fi engaged Halborn to conduct a security audit on their smart contracts beginning on February 9th and ending on February 18th. The security assessment was scoped to the smart contracts provided to the Halborn team.
The team at Halborn was provided two weeks for the engagement and assigned a full-time security engineer to audit the security of the smart contract. The security engineer is a blockchain and smart-contract security expert with advanced penetration testing, smart-contract hacking, and deep knowledge of multiple blockchain protocols.
The purpose of this audit is to:
Ensure that smart contract functions operate as intended
Identify potential security issues with the smart contracts
In summary, Halborn identified some improvements to reduce the likelihood and impact of risks, which were mostly addressed by Astroport.fi. The main one being the lack of validation during Bridge removal.
Halborn performed a combination of manual review of the code and automated security testing to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of the smart contract audit. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of smart contracts and can quickly identify items that do not follow security best practices. The following phases and associated tools were used throughout the term of the audit:
Research into architecture, purpose, and use of the platform.
Manual code read and walkthrough.
Manual assessment of use and safety for the critical Rust variables and functions in scope to identify any contracts logic related vulnerability.
Fuzz testing (Halborn custom fuzzing tool)
Checking the test coverage (cargo tarpaulin)
Scanning of Rust files for vulnerabilities (cargo audit) \newline
Code repository: tokenomics-maker
\begin{enumerate} \item CosmWasm Maker Smart Contract \begin{enumerate} \item Commit ID: \href{https://github.com/astroport-fi/astroport-core/tree/bd8f8599e1b1867b6e6a005bc4cef209699683e6/contracts/tokenomics/maker}{bd8f8599e1b1867b6e6a005bc4cef209699683e6} \item Contract in scope: \begin{enumerate} \item Maker contract \end{enumerate} \end{enumerate} \end{enumerate}
Out-of-scope: External libraries and financial related attacks.
Critical
0
High
0
Medium
0
Low
1
Informational
3
Impact x Likelihood
HAL-01
HAL-02
HAL-03
HAL-04
| Security analysis | Risk level | Remediation Date |
|---|---|---|
| LACK OF VALIDATION UPON BRIDGE REMOVAL | Low | Solved - 02/28/2022 |
| MISUSE OF HELPER METHODS | Informational | Solved - 02/28/2022 |
| UNCHECKED MATH | Informational | - |
| OVERFLOW CHECKS NOT SET FOR PROFILE RELEASE | Informational | Acknowledged |
// Low
AssetInfo instances do not normalize capitalization, taking as different assets ULUNA and uluna. When using the update_bridges function to remove bridges, if a different capitalization was used on the elements of the remove parameter the operation resulted in no modifications being done without the sender receiving any error feedback, as those won't be found in the current list of bridges.
In case the owner unwillingly included incorrect assets to remove the bridge, the undesired bridge would still be available to perform token swapping by the contract, potentially affecting the contract's tokenomics.
// remove old bridges
if let Some(remove_bridges) = remove {
for asset in remove_bridges {
BRIDGES.remove(deps.storage, asset.to_string());
}
}
SOLVED: The issue was fixed with the above recommendation in commit b7fa67c4d2429e61139331717639cd8f50cb1629.
// Informational
The use of the unwrap function is very useful for testing environments because a value is forcibly demanded to get an error (aka panic!) if the "Option" does not have "Some" value or "Result". Nevertheless, leaving unwrap functions in production environments is a bad practice because not only will this cause the program to crash out, or panic!, but also no helpful messages are shown to help the user solve or understand the reason of the error.
contracts/tokenomics/maker/src/contract.rs:801: let (asset, bridge) = item.unwrap();
contracts/tokenomics/maker/src/contract.rs:802: (String::from_utf8(asset).unwrap(), bridge.to_string())
SOLVED: The issue was fixed with the above recommendation in commit b7fa67c4d2429e61139331717639cd8f50cb1629.
// Informational
// Informational
While the overflow-checks parameter is set to true in profile.release and implicitly applied to all contracts and packages from in workspace, it is not explicitly enabled in Cargo.toml file for each individual package, which could lead to unexpected consequences if the project is refactored.
contracts/tokenomics/maker/Cargo.toml
ACKNOWLEDGED: Astroport acknowledged this finding.
Halborn used automated security scanners to assist with detection of well-known security issues and vulnerabilities. Among the tools used was cargo audit, a security scanner for vulnerabilities reported to the RustSec Advisory Database. All vulnerabilities published in https://crates.io are stored in a repository named The RustSec Advisory Database. cargo audit is a human-readable version of the advisory database which performs a scanning on Cargo.lock. Security Detections are only in scope. To better assist the developers maintaining this code, the auditors are including the output with the dependencies tree, and this is included in the cargo audit output to better know the dependencies affected by unmaintained and vulnerable crates.
\begin{center} \begin{tabular}{|l|p{2cm}|p{9cm}|} \hline \textbf{ID} & \textbf{package} & \textbf{Short Description} \ \hline \href{https://rustsec.org/advisories/RUSTSEC-2020-0025}{RUSTSEC-2020-0025} & bigint & biginit is unmaintained, use uint instead \ \hline \end{tabular} \end{center}
Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed