Halborn Logo

Brokkr Protocol P2 Contracts - Brokkr


Prepared by:

Halborn Logo

HALBORN

Last Updated 04/26/2024

Date of Engagement by: March 21st, 2022 - March 30th, 2022

Summary

100% of all REPORTED Findings have been addressed

All findings

2

Critical

0

High

0

Medium

1

Low

0

Informational

1


1. AUDIT SUMMARY

Brokkr engaged Halborn to conduct a security assessment on CosmWasm smart contracts beginning on March 21st, 2022 and ending on March 30th, 2022.

The security engineers involved on the audit are blockchain and smart-contract security experts with advanced penetration testing, smart-contract hacking, and deep knowledge of multiple blockchain protocols.

The purpose of this audit is to achieve the following:

    • Ensure that smart contract functions work as intended.

    • Identify potential security issues with the smart contracts.

In summary, Halborn identified some improvements to reduce the likelihood and impacts of the risks, which were mostly addressed by the Brokkr team. The main ones are the following:

    • Split owner address transfer functionality to allow transfer to be completed by recipient.

    • Use checked arithmetical operations.

External threats, such as financial related attacks, oracle attacks, and inter-contract functions and calls should be validated for expected logic and state.

2. TEST APPROACH & METHODOLOGY

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

3. SCOPE

\begin{enumerate} \item CosmWasm Smart Contracts \begin{enumerate} \item Repository: \href{https://github.com/block42-blockchain-company/brotocol-token-contracts/tree/main}{brotocol-token-contracts} \item Commit ID: \href{https://github.com/block42-blockchain-company/brotocol-token-contracts/tree/6e5b287382d1c3c29d568851ce3038ffff7407a3}{6e5b287382d1c3c29d568851ce3038ffff7407a3} \item Contracts in scope: \begin{enumerate} \item bbro-token \item bbro-minter \item distributor-v1 \end{enumerate} \end{enumerate} \end{enumerate}

Out-of-scope: External libraries and financial related attacks

4. RISK METHODOLOGY

Vulnerabilities or issues observed by Halborn are ranked based on the risk assessment methodology by measuring the LIKELIHOOD of a security incident and the IMPACT should an incident occur. This framework works for communicating the characteristics and impacts of technology vulnerabilities. The quantitative model ensures repeatable and accurate measurement while enabling users to see the underlying vulnerability characteristics that were used to generate the Risk scores. For every vulnerability, a risk level will be calculated on a scale of 5 to 1 with 5 being the highest likelihood or impact.
RISK SCALE - LIKELIHOOD
  • 5 - Almost certain an incident will occur.
  • 4 - High probability of an incident occurring.
  • 3 - Potential of a security incident in the long term.
  • 2 - Low probability of an incident occurring.
  • 1 - Very unlikely issue will cause an incident.
RISK SCALE - IMPACT
  • 5 - May cause devastating and unrecoverable impact or loss.
  • 4 - May cause a significant level of impact or loss.
  • 3 - May cause a partial impact or loss to many.
  • 2 - May cause temporary impact or loss.
  • 1 - May cause minimal or un-noticeable impact.
The risk level is then calculated using a sum of these two values, creating a value of 10 to 1 with 10 being the highest level of security risk.
Critical
High
Medium
Low
Informational
  • 10 - CRITICAL
  • 9 - 8 - HIGH
  • 7 - 6 - MEDIUM
  • 5 - 4 - LOW
  • 3 - 1 - VERY LOW AND INFORMATIONAL

5. SCOPE

Out-of-Scope: New features/implementations after the remediation commit IDs.

6. Assessment Summary & Findings Overview

Critical

0

High

0

Medium

1

Low

0

Informational

1

Impact x Likelihood

HAL-01

HAL-02

Security analysisRisk levelRemediation Date
PRIVILEGED ADDRESS CAN BE TRANSFERRED WITHOUT CONFIRMATIONMediumSolved - 03/21/2022
UNCHECKED ARITHMETICAL OPERATIONS CAN CAUSE PANICInformationalAcknowledged

7. Findings & Tech Details

7.1 PRIVILEGED ADDRESS CAN BE TRANSFERRED WITHOUT CONFIRMATION

// Medium

Description

Incorrect use of the update_config function in contracts can set owner to have an invalid address and inadvertently lose control of the contracts, which cannot be undone in any way. Currently, the contract owner can change the owner address using the aforementioned function in a single transaction and without confirmation from the new address.

The affected smart contracts are the following:

  • epoch-manager
  • distributor-v1

Code Location

contracts/distributor-v1/src/commands.rs

let mut config = load_config(deps.storage)?;

if let Some(owner) = owner {
    config.owner = deps.api.addr_canonicalize(&owner)?;
}

contracts/bbro-minter/src/commands.rs

let mut config = load_config(deps.storage)?;

if let Some(owner) = owner {
    config.owner = deps.api.addr_canonicalize(&owner)?;
}
Score
Impact: 4
Likelihood: 2
Recommendation

SOLVED: The issue was fixed in commit 79549c38936e99a89a1fa7aa7e38456032f47389.

7.2 UNCHECKED ARITHMETICAL OPERATIONS CAN CAUSE PANIC

// Informational

Description

When calculating the distribution of rewards, the distributor-v1 contract performs a division without checking that the denominator is not zero. In the event that epoch_manager is misconfigured and has an epoch length of 0 blocks (that can happen because the epoch manager does not check for that condition), the distribute function would panic, leaving the user without a precise error message for your failed transaction.

Code Location

contracts/distributor-v1/src/commands.rs,

// query epoch from epoch_manager contract
let epoch_blocks = query_epoch_info(
    &deps.querier,
    deps.api.addr_humanize(&config.epoch_manager_contract)?,
)?
.epoch;

// distribute rewards only for passed epochs
let blocks_since_last_distribution = env.block.height - state.last_distribution_block;
let passed_epochs = blocks_since_last_distribution / epoch_blocks;
if passed_epochs == 0 {
    return Err(ContractError::NoRewards {});
}
Score
Impact: 1
Likelihood: 1
Recommendation

\textbf{ACKNOWLEDGED:} The Brokkr team acknowledged this finding. They also stated that since epoch_blocks is a primitive type, there is no need to convert it to a wrapped as that would break the whole protocol.

8. Automated Testing

AUTOMATED ANALYSIS

Description

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. All the vulnerabilities shown here were already disclosed in the previous report. However, to better help developers maintain this code, the auditors include the output with the dependency tree, and this is included in the cargo audit output to better understand the dependencies affected by unmaintained and vulnerable crates.

\begin{center} \begin{tabular}{|l|p{3.5cm}|p{6.5cm}|} \hline \textbf{ID} & \textbf{package} & \textbf{Short Description} \ \hline \href{https://rustsec.org/advisories/RUSTSEC-2020-0025}{RUSTSEC-2020-0025} & bigint & bigint 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.

© Halborn 2024. All rights reserved.