Sienna.Network AMM Protocol (Updated code) - Sienna.Network

Prepared by:

HALBORN

Last Updated 04/26/2024

Date of Engagement by: February 7th, 2022 - February 16th, 2022

Summary

75% of all REPORTED Findings have been addressed

All findings

Critical

High

Medium

Low

Informational

1. Introduction
2. Audit summary
3. Test approach & methodology
4. Scope
5. Risk methodology
6. Scope
7. Assessment summary & findings overview
8. Findings & Tech Details

8.1 Possibility to create pools with the same pair
8.2 Exchange swap fees are not stable
8.3 Data leakage attacks by analyzing metadata of contracts usage
8.4 Mismatch between assertion and error description

9. Automated Testing

1. INTRODUCTION

Sienna.Network engaged Halborn to conduct a security audit on their smart contracts beginning on February 7th, 2022 and ending on February 16th, 2022. The security assessment was scoped to the smart contracts provided to the Halborn team.

2. AUDIT SUMMARY

The team at Halborn was provided 1 week 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
Review codebase changes since last audit

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

Ensure that the factory cannot create a pair of the same token by detecting same addresses with lower/upper case.
Take fees into account in the computation of an exchange swap.
Protect the users from data leakage by applying best practices relative to Secret Network security model.

3. 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

4. SCOPE

\begin{enumerate} \item CosmWasm Smart Contracts \begin{enumerate} \item Repository: \href{https://git.sienna.network/SiennaNetwork/contracts}{https://git.sienna.network/SiennaNetwork/contracts} \item Commit ID: \href{https://git.sienna.network/SiennaNetwork/contracts/src/commit/1cea0c7989825ee8ca2279942dedf4ca29a0f880/contracts/amm}{1cea0c7989825ee8ca2279942dedf4ca29a0f880} \item Contracts in scope: \begin{enumerate} \item exchange \item factory \end{enumerate} \end{enumerate} \end{enumerate}

It is worth noting that the results of this audit are a complement to the information provided in a previous report for the security audit performed to the codebase with commit id 13ce1ac9728e16b3d79d64caca603fe029882371.

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

5. 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

6. SCOPE

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

7. Assessment Summary & Findings Overview

Critical

High

Medium

Low

Informational

Impact x Likelihood

Likelihood

Impact

HAL-01

HAL-02

HAL-03

HAL-04

Security analysis	Risk level	Remediation Date
POSSIBILITY TO CREATE POOLS WITH THE SAME PAIR	High	Risk Accepted
EXCHANGE SWAP FEES ARE NOT STABLE	Informational	-
DATA LEAKAGE ATTACKS BY ANALYZING METADATA OF CONTRACTS USAGE	Informational	Acknowledged
MISMATCH BETWEEN ASSERTION AND ERROR DESCRIPTION	Informational	Acknowledged

8. Findings & Tech Details

8.1 POSSIBILITY TO CREATE POOLS WITH THE SAME PAIR

// High

Description

The AMM module allow users to create pairs (called exchanges) from native or custom assets by calling create_exchange function on factory contract. It relies on the token_type PartialEq implementation, that allows users to pair the same asset twice, which generates unexpected situations, e.g.: a user could withdraw more tokens than his fair share and affect other users in the pool.

The verification asserts that both tokens addresses are not exactly equal, but a lowercase version of the address will be considered different from the uppercase one.

Code Location

libraries/amm-shared/src/token_type.rs

impl<A: PartialEq> PartialEq for TokenType<A> {
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (
                Self::CustomToken { contract_addr: l_contract_addr, .. },
                Self::CustomToken { contract_addr: r_contract_addr, .. }
            ) => l_contract_addr == r_contract_addr,
            (
                Self::NativeToken { denom: l_denom },
                Self::NativeToken { denom: r_denom }
            ) => l_denom == r_denom,
            _ => false
        }
    }
}

Score

Impact: 4
Likelihood: 4

Recommendation

\textbf{RISK ACCEPTED:} The Sienna.Network team accepted the risk for this finding.

8.2 EXCHANGE SWAP FEES ARE NOT STABLE

// Informational

Description

Finding description placeholder

Score

Impact:
Likelihood:

8.3 DATA LEAKAGE ATTACKS BY ANALYZING METADATA OF CONTRACTS USAGE

// Informational

Description

Depending on a contract's implementation, an attacker could deanonymize information about the contract and its clients. In all the following scenarios, assume that an attacker has a local full node in its control.

For example, it is possible for an attacker to create a list of every account that performs a swap. This attack is based on the length of the (encrypted) message sent to the attacker's node: there is a significant difference of size between the only two queries. On one side, pair_info is short and doesn't take any parameter. On the other side, swap_simulation is longer and takes a parameter.

This means that the inputs for queries on this contract would look like:

"pair_info"
{"swap_simulation": {"offer": some data}}

Therefore, the attacker (a validator node or internet provider) can distinguish what query was called given the length of the message. If the message is long, the user might have asked for a swap simulation and could perform a swap transaction after that.

Code Location

contracts/amm/exchange/src/contract.rs

pub fn query<S: Storage, A: Api, Q: Querier>(deps: &Extern<S, A, Q>, msg: QueryMsg) -> QueryResult {
    match msg {
        QueryMsg::PairInfo => {
            let config = load_config(deps)?;

            let balances = config.pair.query_balances(
                &deps.querier,
                config.contract_addr,
                config.viewing_key.0,
            )?;
            let total_liquidity = query_liquidity(&deps.querier, &config.lp_token_info)?;

            to_binary(&QueryMsgResponse::PairInfo {
                liquidity_token: config.lp_token_info,
                factory: config.factory_info,
                pair: config.pair,
                amount_0: balances[0],
                amount_1: balances[1],
                total_liquidity,
                contract_version: CONTRACT_VERSION,
            })
        }
        QueryMsg::SwapSimulation { offer } => {
            let config = load_config(deps)?;
            to_binary(&swap_simulation(deps, config, offer)?)
        }
    }
}

Score

Impact: 1
Likelihood: 1

Recommendation

\textbf{ACKNOWLEDGED:} The Sienna.Network team acknowledged this finding.

8.4 MISMATCH BETWEEN ASSERTION AND ERROR DESCRIPTION

// Informational

Description

In the exchange contract, the description of the slippage check and the code do not match:

Description explains that slippage tolerance must be bewteen 0.1 and 0.9.
Verification ensures that slippage is contained between 0 and 1 excluded.

The confusion between the code and the description could confuse users interacting with the contract.

Code Location

contracts/amm/exchange/src/contract.rs

if slippage.is_zero() || slippage >= Decimal256::one() {
    return Err(StdError::generic_err(
        format!("Slippage tolerance must be between 0.1 and 0.9, got: {}", slippage))
    );
}

Score

Impact: 1
Likelihood: 1

Recommendation

\textbf{ACKNOWLEDGED:} The Sienna team acknowledged this finding and also stated that if a mistake is made for whatever reason, it can quickly be corrected.

9. 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 vulnerabilities shown here were already disclosed in the above report. However, 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{3cm}|p{8cm}|} \hline \textbf{ID} & \textbf{package} & \textbf{Short Description} \ \hline \href{https://rustsec.org/advisories/RUSTSEC-2020-0159}{RUSTSEC-2020-0159} & chrono & Potential segfault in localtime\_r \ \hline \href{https://rustsec.org/advisories/RUSTSEC-2021-0076}{RUSTSEC-2021-0076} & libsecp256k1 & libsecp256k1 allows overflowing \ \hline \href{https://rustsec.org/advisories/RUSTSEC-2020-0071}{RUSTSEC-2020-0071} & time & Potential segfault in the time crate \ \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.

1. Introduction
2. Audit summary
3. Test approach & methodology
4. Scope
5. Risk methodology
6. Scope
7. Assessment summary & findings overview
8. Findings & Tech Details