INTRODUCTION

BlockApex (Auditor) was contracted by VoirStudio (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which started from 26th Jan 2022. 

Name: Unipilot-V2
Auditor: Moazzam Arif | Kaif Ahmed | Muhammad Jarir Uddin
Platform: Ethereum/Solidity
Type of review: Manual Code Review | Automated Code Review
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/VoirStudio/unipilot-v2/tree/revamp-structure
White paper/ Documentation: https://unipilot.gitbook.io/unipilot/
Document log:
Initial Audit: 14th Feb 2022 (complete)
Quality Control: 14th - 22nd March 2022
Final Audit: 26th March 2022 (Complete)

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

Unipilot is an automated liquidity manager designed to maximize ”in-range” intervals for capital through an optimized rebalancing mechanism of liquidity pools. Unipilot V2 also detects the volatile behavior of the pools and pulls liquidity until the pool gets stable to save the pool from impairment loss.

System Architecture

The protocol is built to support multiple dexes (decentralized exchanges) for liquidity management. Currently it supports only Uniswap v3’s liquidity. In future, the protocol will support other decentralized exchanges like Sushiswap (Trident). The architecture is designed to keep in mind the future releases.

The protocol has 6 main smart contracts and their dependent libraries.

UnipilotActiveFactory.sol

The smart contract is the entry point in the protocol. It allows users to create a vault if it's not present on protocol. Nevertheless Active vaults can only be created by governance.

UnipilotPassiveFactory.sol

The smart contract is the entry point in the protocol. It allows users to create a vault if it's not present on protocol. However passive vaults can be created by anyone.

UnipilotActiveVault.sol

Vault contract allows users to deposit, withdraw, readjustLiquidity and collect fees on liquidity. It mints an LPs to its users representing their individual shares. It also has a pullLiquidity function if liquidity is needed to be pulled.

UnipilotPassiveVault.sol

PassiveVault contract allows users to deposit, withdraw, readjustLiquidity and collect fees on liquidity. It mints an LPs to its users representing their individual shares.

UnipilotStrategy.sol

The smart contract to fetch and process ticks’ data from Uniswap. It also decides the bandwidth of the ticks to supply liquidity.

UnipilotMigrator.sol

The smart contract aids to migrate users liquidity from other Uniswap V3 Liquidity Optimizer Protocols to Unipilot V2 Protocol.

Methodology & Scope

The codebase was audited in an iterative process. Fixes were applied on the way and updated contracts were examined for more bugs. We used a combination of static analysis tool (slither) and Automated testing tool (Foundry) which indicated some of the critical bugs in the code. We also did manual reviews of the code to find logical bugs, code optimizations, solidity design patterns, code style and the bugs/ issues detected by automated tools.

Privileged Roles

In a production environment, the unipilot protocol sets the address for a governance that exercises a privileged position over the factory and vault contracts in the system. The governor has the power to initiate a transfer of the governor role to a new address.

The governance address is capable of executing a set of actions including:

The operator address has following activities it can be used for:

AUDIT REPORT

Executive Summary

The analysis indicates that some of the functionalities in the contracts audited are working properly.

Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX, Surya and Slither. All the flags raised were manually reviewed and re-tested. 

Our team found: 

# of issues Severity of the risk
1Critical Risk issue(s)
2High Risk issue(s)
2Medium Risk issue(s)
7Low Risk issue(s)
11Informatory issue(s)

Findings

#FindingsRiskStatus
1.Deposit ether with zero valueCriticalFixed
2.init() should have only once checkHighFixed
3.Pulled liquidity rationaleHighAcknowledged
4.Unoptimized user liquidity is held in a vault.MediumAcknowledged
5.Deposit should have non-reentrant checks placed in both vaultsMediumFixed
6.Zero Address checks not placed in contract constructors()LowAcknowledged
7.Safecast in UnipilotPassiveVault.sol line 232 & 233 for `swapPercentage`LowAcknowledged
8.The storage Layout is unoptimized.LowAcknowledged
9.Check max cap for indexFundPercentage and swapPercentage LowFixed
10.Deposit() function optimizationLowFixed
11.toggleWhitelistAccount() redundant.LowPending
12.SortWeth() optimizationLowAcknowledged
13.No natspec documentation in UnipilotActiveVault.solInformatoryAcknowledged
14.Mark token0 and token1 as immutable in UnipilotActivevault.sol and UnipilotPassiveVault.solInformatoryPartially Fixed
15.onlyGovernance() modifier in passive vault contractInformatoryFixed
16._WETH address should be hardcoded before production wherever necessaryInformatoryAcknowledged
17.Factory function createVault() optimization version.InformatoryAcknowledged
18.Assignment of Params in order to receive the function signature.InformatoryAcknowledged
19.Order of functions in solidity style guides.InformatoryAcknowledged
20.uint256 can be cheaper than uint8InformatoryFixed
21.pullLiquidity() is vulnerable in its current executionInformatoryFixed
22.Spelling mistakes in function signatures.InformatoryAcknowledged
23.Mark private functions as internalInformatoryFixed

Critical risk issues

1. Deposit ether with zero value.

Description: 

If a deposit of tokens with ether as one is made, all the while the contract has pulled liquidity into the vault, the user making a deposit with 0 value can use the vault’s ether to execute a successful transaction.

Remedy:

Introduce a RefundETH() function that ensures a proper transfer of value either be in ethers or an ERC compatible version (WETH)

Status: 

Fixed

High risk issues

1. init() should have only once check

Description: 

Calling init() any time after the first time it has been called, can lead to permanent loss of position at uniswap V3.

function init() external onlyGovernance {
        int24 _tickSpacing = tickSpacing;
        int24 baseThreshold = _tickSpacing * getBaseThreshold();
        (, int24 currentTick, ) = pool.getSqrtRatioX96AndTick();

        int24 tickFloor = UniswapLiquidityManagement.floor(
            currentTick,
            _tickSpacing
        );

        ticksData.baseTickLower = tickFloor - baseThreshold;
        ticksData.baseTickUpper = tickFloor + baseThreshold;

        UniswapLiquidityManagement.checkRange(
            ticksData.baseTickLower,
            ticksData.baseTickUpper
        );
    }

Remedy:

There should be an onlyOnce modifier or a variable handling (as locks) that ensures init is never called again.

Status:

Fixed as per BlockApex recommendation.

2. Pulled liquidity rationale

Description: 

Considering the scenario where the vault has pulled Liquidity with the intention of depositing it back to Uniswap V3 when the pool is relatively less volatile; the smart contract code assumes a rational behavior to override checkDeviation modifier by manually modifying ticks through strategy using onlyGovernance functions. But the code does not guarantee any logic to push liquidity back to v3 in a safe manner

Remedy:

Debatable.

Status:

Acknowledge

Developer’s response:

Governance will manually update deviation in case of price volatility.

Medium risk issues

  1. Unoptimized user liquidity is held in a vault.

Description: 

In Active vaults if a pool is created on 1-X ratio on Uniswap V3 and a user makes a deposit with 1-1 ratio through the vault, the vault is found to hold the remaining amount of the token initially at the price of X, giving users a complete share with proportion of the deposited amount while the remaining amount of user sits inactive within the vault.

Remedy: 

The vault must ensure that the amounts provided by a user are equal to the amounts of tokens actually deposited on a Uniswap pool.

Status:

Acknowledged

Developer’s response:

Added rearrange liquidity method.

  1. Deposit should have non reentrant checks placed in both vaults

Description: 

The deposit() function in both vaults contract, that is, the UnipilotActiveVault and the UnipilotPassiveVault does not contain a non-reentrant modifier which is a standard practice to prevent any adversarial intent related to the reentrancy exploits.

Remedy:

A good industry practice requires that the deposit() function executes with a non-reentrant modifier; this modifier should be placed to ensure security as the deposit marks external calls through linked libraries to deposit values to the Uniswap V3.

Status:

Fixed as per BlockApex recommendation. 

Low-risk issue

  1. Zero Address checks not placed in contract constructors()

Description: 

Constructor() does not contain checks for accepting params of address type whether an address is zero or not.

Remedy:

Since the constructor accepts an address from an argument, there should be a zero address check to ensure the functionality. These checks should be placed in almost every contract: Unipilot Factory , Unipilot Strategy , Unipilot Migration etc.

Status:

Acknowledged

  1. Safecast in UnipilotPassiveVault.sol line 241 & 242 for `swapPercentage`

Description: 

In UnipilotPassiveVault.sol the readjustLiquidity() reads the swapPercentage variable in Line 238 of the contract to calculate the amountSpecified variable in Lines 241-242, this Math is unsafe as the calculation is executed with different types for each param. 

    if (amount0 == 0 || amount1 == 0) {
            bool zeroForOne = amount0 > 0 ? true : false;

            (, , , , uint8 swapPercentage) = getProtocolDetails();

            int256 amountSpecified = zeroForOne
                ? int256(FullMath.mulDiv(amount0, swapPercentage, 100))
                : int256(FullMath.mulDiv(amount1, swapPercentage, 100));

            pool.swapToken(address(this), zeroForOne, amountSpecified);
        }

Remedy:

Use safecasting for all type variables on lines 232-233 to ensure a seamless execution of the desired arithmetics.

Status:

Acknowledged

Developer’s response:

Percentage calculation is correct with this method. (Well tested)

3. Storage Layout is unoptimized.

Description:

Variable tight packing is strongly recommended for both vaults and factories in state variable declaration as the contracts are composed in order that is gas-consuming.

Remedy:

A solidity design pattern ‘Tight variable Packing’ ensures that the smart contract is optimized to execute efficiently within the EVM environment. 

Status:

Fixed as per BlockApex recommendation.

Status:

Slots are arranged bitwise now.

4. Check max cap for indexFundPercentage and swapPercentage.

Description: 

In setUnipilotDetails() the param indexFundPercentage is checked to receive a lowest value greater than zero.

  function setUnipilotDetails(
        address _strategy,
        address _indexFund,
        uint8 _indexFundPercentage
    ) external onlyGovernance {
        require(_strategy != address(0) && _indexFund != address(0));
        require(_indexFundPercentage > 0);
        strategy = _strategy;
        indexFund = _indexFund;
        indexFundPercentage = _indexFundPercentage;
    }

Remedy:

Ensure a check placed to bound the maximum value for the indexFundPercentage

Status:

Fixed

5. Deposit() function optimization.

Description: 

In UnipilotActiveVault.sol and UnipilotPassiveVault.sol, Users can call deposit() with zero amounts of both tokens and the function executes until the end.

function deposit(
        uint256 amount0Desired,
        uint256 amount1Desired,
        address recipient
    )
        external
        payable
        override
        returns (
            uint256 lpShares,
            uint256 amount0,
            uint256 amount1
        )
    {
        address sender = _msgSender();

        (lpShares, amount0, amount1) = pool.computeLpShares(
            true,
            amount0Desired,
            amount1Desired,
            _balance0(),
            _balance1(),
            totalSupply(),
            ticksData
        );

Remedy:

Function should check for zero value for both input args in the deposit() function in vaults contract.

Status:

Fixed

6. toggleWhitelistAccount() redundant.

Description:

toggleWhitelistAccount() can toggle the gov off in a redundant call of the same function to whitelist itself back. 

 function toggleWhitelistAccount(address _address) external onlyGovernance {
        require(_address != address(0));
        isWhitelist[_address] = !isWhitelist[_address];
    }

Remedy:

Ensure the address is checked to not allow governance to be toggled for whitelist.

Status:

Acknowledged.

7. _SortWeth() optimization

 function _sortWethAmount(
        address _token0,
        address _token1,
        uint256 _amount0,
        uint256 _amount1
    )
        private
        pure
        returns (
            address tokenAlt,
            uint256 altAmount,
            address tokenWeth,
            uint256 wethAmount
        )
    {
        // (
        //     address tokenA,
        //     address tokenB,
        //     uint256 amountA,
        //     uint256 amountB
        // ) = _token0 == WETH
        //         ? (_token0, _token1, _amount0, _amount1)
        //         : (_token0, _token1, _amount1, _amount0);

        (tokenAlt, altAmount, tokenWeth, wethAmount) = _token0 == WETH
            ? (_token1, _amount1, _token0, _amount0)
            : (_token0, _amount0, _token1, _amount1);
    }

Description: 

This function’s logic can be concise. The remedy, tested against the required logic, is mentioned as a code snippet in the screenshot above.

Status:

Acknowledged

Informatory issues

1. No NatSpec documentation

Description: 

NatSpec documentation is an essential part of smart contract readability; it is therefore advised that all contracts and following files contain proper explanatory commenting;

Status:

Acknowledged

Developer’s response:

Completed netspec for all contracts.

2. Mark token0 and token1 as immutable in UnipilotActivevault.sol and UnipilotPassiveVault.sol

Description: 

State variables containing the address of tokens should be marked as immutable as the constructor locks the values for each after deployment.

Status:

Partial Fixed.

Developer’s response:

PassiveVault used immutables however active vaults don’t due to size issues.

3. onlyGovernance() modifier in passive vault contract


Description: 

The onlyGovernance modifier in the Passive Vault contract remains unused within the contract.


Status:

Fixed as per BlockApex recommendation.

4. _WETH address should be hardcoded before production wherever necessary

Description: 

Address of the WETH token contract is passed as a constructor param in both Factories which can be optimized by hardcoding the actual address of _WETH in the final deployment of the production environment.

constructor(
        address _pool,
        address _unipilotFactory,
        address _WETH,
        address governance,
        string memory _name,
        string memory _symbol
    ) ERC20Permit(_name) ERC20(_name, _symbol) {
        WETH = _WETH;
        unipilotFactory = IUnipilotFactory(_unipilotFactory);
        pool = IUniswapV3Pool(_pool);
        token0 = IERC20(pool.token0());
        token1 = IERC20(pool.token1());
        fee = pool.fee();
        tickSpacing = pool.tickSpacing();
        _operatorApproved[governance] = true;
    }

Status:

Acknowledged

5. Factory function createVault() optimized version

Description: 

createVault() is found to be optimized if it executes in the following recommended pattern: 

Current Implementation:

function createVault(
        address _tokenA,
        address _tokenB,
        uint24 _fee,
        uint160 _sqrtPriceX96,
        string memory _name,
        string memory _symbol
    ) external override onlyGovernance returns (address _vault) {
        require(_tokenA != _tokenB);
        (address token0, address token1) = _tokenA < _tokenB
            ? (_tokenA, _tokenB)
            : (_tokenB, _tokenA);
        require(vaults[token0][token1][_fee] == address(0));
        address pool = uniswapFactory.getPool(token0, token1, _fee);

        if (pool == address(0)) {
            pool = uniswapFactory.createPool(token0, token1, _fee);
            IUniswapV3Pool(pool).initialize(_sqrtPriceX96);
        }

        _vault = address(
            new UnipilotActiveVault{
                salt: keccak256(abi.encodePacked(_tokenA, _tokenB, _fee))
            }(pool, address(this), WETH, governance, _name, _symbol)
        );

        isWhitelist[_vault] = true;
        vaults[token0][token1][_fee] = _vault;
        vaults[token1][token0][_fee] = _vault; // populate mapping in the reverse direction
        emit VaultCreated(token0, token1, _fee, _vault);
    }

Status:

Acknowledged

6. Assignment of Params in order to receive the function signature.

Description: 

In all four contracts of vault and factory the constructor receives arguments in order which is out-of-sync to the one being assigned, reducing the code readability. Ensure param values and actual assignments are in sync for better code readability.

constructor(
        address _pool,
        address _unipilotFactory,
        address _WETH,
        address governance,
        string memory _name,
        string memory _symbol
    ) ERC20Permit(_name) ERC20(_name, _symbol) {
        WETH = _WETH;
        unipilotFactory = IUnipilotFactory(_unipilotFactory);
        pool = IUniswapV3Pool(_pool);
        token0 = IERC20(pool.token0());
        token1 = IERC20(pool.token1());
        fee = pool.fee();
        tickSpacing = pool.tickSpacing();
        _operatorApproved[governance] = true;
    }

Status:

Acknowledged

7. Order of functions as in solidity Style Guide

Description: 

Receive() and Fallback() should be moved on top, below constructor; following the solidity design patterns

Status:

Fixed

8. uint256 can be cheaper than uint8

Description: 

Uint8 is proved to be more costly than uint256 variables in a number of scenarios, where a better and optimized variable packing for uint8 variables is recommended or replaced with uint256/ uint64/ uint24 type vars.

Status:

Fixed

9. pullLiquidity() is vulnerable in its current execution

Description: 

The pullLiquidity(address _recipient) method is vulnerable to some extent, holding potential for mal-intent or permanent loss of value. Checking for the address argument as not another whitelisted vault can ensure no accidental and permanent loss of tokens happen.

Status:

Pending

Developer’s response:

Vaults will be whitelisted only for the execution of pull liquidity (when needed) soon after execution that vault will be blacklisted in order to avoid accidentally sending tokens to other active vaults.

10. Spelling mistakes in function signatures

Description: 

In the UnipilotMigrator.sol file,
migrateUnipilotLiquididty() and  _refundRemainingLiquidiy() are spelled wrong, causing readability issues as well as creating the wrong function signature.

Status:

Fixed 

11. Mark private functions as internal

Description: 

In the UnipilotMigrator.sol file,
_sortWethAmount() and _addLiquidityUnipilot() are private, which are gas costly.

Status:

Fixed as per BlockApex recommendation.

DISCLAIMER 

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.

INTRODUCTION

BlockApex (Auditor) was contracted by Dafi Protocol (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which took place from 16th Dec 2021 to 14th Jan 2022. 

Name: Dafi Super Staking V2
Auditor: Moazzam Arif | Muhammad Jarir Uddin
Platform: Ethereum/Solidity
Type of review: Staking, Mathematics, Oracle feeds
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/DAFIProtocol/dDAFI/tree/testing
White paper/ Documentation: https://docs.dafiprotocol.io/super-staking/overview-super-staking
Document log: Initial Audit: 31st December 2021 (complete)
Formal verification using property-based testing: 15th January 2022
Final Audit: 17th January 2022.

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/ vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

A comprehensive explanation of Staking module V2 for the Dafi Protocol is present in the official documentation and can be viewed at: https://docs.dafiprotocol.io/super-staking/super-staking-v2

Dafi Protocol Super Staking V2 is an update to the V1 module released earlier this year (2021) in the month of July. Super Staking V2 claims to offer a more stable APY rate and enhanced distribution of dDAFI rewards by modifying the math behind reward calculation to rather depend only on accumulated amounts of reward every time demand factor changes to calculating the rewards of users as the sum of all rewards in the past adjusted to the latest demand factor. 

Aside from the new reward formula, V2 also holds a couple of security improvements where demand factor is now enumerated using the latest price and is fortified by introduction of a delay i.e. a variance tolerance mechanism to ultimately prevent a sudden change in price. This new demand factor is supported by a TWAP calculation to make the price curve less steeper. Although strict monitoring is required, in cases of hackable oracle feeds, the protocol is able to recover from any kind of exploits by updating to an entirely new oracle service.

System Architecture

Codebase:

The system consists of 5 main smart contracts (namely: Staking Manager V2, Staking Database, Rebase Engine, Network Demand, Token Pool) and supplied with external data through 2 more contracts of Price Feeds and TVL Feeds.

Note: All of the contracts mentioned above contain onlyOwner modifiers to add, set and/or update configurations.

SHA-256 fingerprints of Contracts included:

Static-Analysis summary

Methodology & Scope

Audit log

Manual Review: The audit launched with a recon phase where a manual code review was conducted to clarify the layers of understanding of the complexities and the general flow of the program. We started by reviewing the two main contracts against common solidity flaws. After the reconnaissance phase we wrote unit-test cases to ensure that the functions are performing their intended behavior. Then we began with the line-by-line manual code review.

Property Testing: From the reconnaissance, a handful of properties were also extracted and labeled as Invariants. In the following days of the audit procedure, the invariants were thoroughly tested against a setup flow of Dafi Staking V2 to ensure each of them held its proper definition.

AUDIT REPORT

Executive Summary

The analysis indicates that some of the functionalities in the contracts audited are poorly-secured. 

Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX, and Slither. All the flags raised were manually reviewed and re-tested. 

Our team found: 

# of issues Severity of the risk
0Critical Risk issue(s)
3High-Risk issue(s)
0Medium Risk issue(s)
1Low-Risk issue(s)
1Informatory issue(s)


Findings

Below is the summary of our findings from the complete audit. This includes any flags raised from the manual/automated code review, behavioral/scenario testing and the properties tested for formal verification. 

S#Properties/FindingsRisk/ImpactStatus
CR-1.Oracle integrityCriticalPassed
HR-1.MAX_DAFI always greater than totaldDafiDistributedHighFailed
HR-2.RewardPool should not have balance if the Program is ended and all users have unstakedHighFailed
HR-3.If a program has ended, users should not be able to stakeHighFailed
LR-1.Reward is claimable even after program has endedLowPassed
LR-2.Unit tests in the testing and main branch of repo at /test/v2.ts should passLowFailed
IR-1.Memory optimization by using uint8 in place of true/ falseInformatory-

Critical-risk

CR-1. Oracle Integrity

Status: Passed

Description

In recent events, Oracles and external data sources have been manipulated to the adversary's benefit. We started off with our focus on the integrity of Oracles (Chainlink in this case) in order to make sure that the data imported into the contracts are not compromised.

The Chainlink oracle library method was tested against the following parameters of a standard audit technique.

function getThePrice() public view override returns (uint) {
        (
        ,
        int price,
        ,
        ,
        ) = priceFeed.latestRoundData();
        return uint(price);
    }

Suggestion: It is advised that a more tested and bonafide data source of TWAPs be considered to make Chainlink’s oracle integrity and reliability as the most optimal.

High-risk

HR-1. MAX_DAFI is always greater than totaldDafiDistributed

Status: Failed

Description

If a program is marked as “ended” before the preset ending duration, that is to say, the elapsed time of the program is less than the program duration, users get revert transactions for staking into the ended program (as seen in the testnet event and confirmed over a fuzzing test scenario with the reason of MAX_DAFI being calculated less than total DAFI distributed till the ending timestamp). This property does not hold at fuzzed inputs;

function test_ConfirmMaxDafiOverflowFuzz(
        uint256 md,
        uint32 pd,
        uint32 bn
    ) public {
        SetupContracts caller = dapping.users(0).setupContract();

        caller.wrapSEtStakingParams(1, md + 1, pd + 1);

        hevm.warp(block.number + bn);

        caller.wrapRebasePool();
        caller.database().getPool();

        uint256 MaxDafi = caller.rebaseEngine().MAX_DAFI();
        uint256 TdDafidist = caller.rebaseEngine().totaldDAFIDistributed();
        
        assertGt(MaxDafi, TdDafidist, 'maxDafi < tdDafiDist; program ended; dDafi overflows');
        
        emit log_named_uint('value of MaxDafi', MaxDafi);
        emit log_named_uint('value of TdDafiDist', TdDafiDist);

    }

Suggestion: It is advised that program duration should always be maintained greater than the time elapsed by supplying checks that assure it.

        if (database.isprogramEnded() && pool.lastUpdatedon > database.getProgramEndedAt()) {
            return;
        } else if (database.isProgramEnded() && pool.lastUpdatedon < database.getProgramEndedAt()) {
            maxTimestampForcalc = database.getProgramEndedAt();
        } else {
            maxTimestampForCalc = block.timestamp;
        }

HR-2. Reward Pool should not have balance if the program has ended and all users have unstaked

Status: Failed

Description

This property claims that the reward pool should be empty after the program has ended and that all the users have unstaked. 

HR-2(a). “markProgramEnded()” by owner’s mistake can lock funds of the reward pool

In case of human error if the program is marked ended the reward pool retains an amount of tokens unclaimed against the percentage of tokens staked in first place.

function test_RewardpoolUnemptyAfterProgramEnded public {
        SetupContracts caller = dapping.users().setupContract();
        hevm.warp (block. timestamp + 5000);
        
        
        caller.wrapSetStakingParams (1, 1000 ether, 12);
        
        uint256 distPoolBalanceBefore = caller.stakingManager().distributionPool(). balance();
        
        hevm.warp(block.timestamp + 99000);
        caller.wrapstake ( 100);
        
        caller.wrapMarkProgramEnded();
        
        hevm warp (block timestamp + 99000);
        caller.wrapunstake(100);
        
        uint256 distPool BalanceAfter = caller.stakingManager().distributionPool(). balance();
        
        assertGt(distPool BalanceBefore, distPoolBalanceAfter);
        
        emit Log named uint('pool Before', distPoolBalanceBefore);
        emit Log named uint('poolAfter', distPoolBalanceAfter);
    }

Suggestion: It is advised to have a refund mechanism if program duration is ended. The refund amount of tokens should be equal to MAX_DAFI - (MDI*totalElapsedTime).

HR-3. If a program has ended, users should not be able to stake

Status: Failed

Description

In a simple scenario test, it was confirmed that Staking was allowed even if the program duration is completed and the program is marked as ended which in the understanding of the Auditor team is an (incomprehensible feature) and can be proved to be a loophole of the system in some scenarios.

Suggestion: The modifier stakeCheck should be supplied with some additional check to ensure monitoring of programs marked as ended not to allow staking.

function test_StakeAfterProgramEnded() public {
        SetupContracts caller = dapping.users().setup Contract();
        hevm.warp(block. timestamp + 500);
        
        // _ms, _md, _pd
        uint256 minStakeDays = 1;
        uint256 maxDafi = 1000 ether;
        uint32 progDuration = 12;
        
        caller.wrapsetStakingParams (minStakeDays, maxDafi, progDuration);
        
        // stakin/ unstaing multiple times for seeding
        for (uint256 i = 0; i < 3; i++) {
            hevm warp (block. timestamp + 90000);
            caller.wrapStake ( 1000000000);
            hevm warp(block. timestamp + 90000);
            caller.wrapunstake ( 1000000000);
        }
        
        caller.wrapMarkProgramEnded();
        
        // stakin/ unstaing multiple times for seeding
        for (uint256 i = 0; i < 3; i++)
            hevm.warp (block. timestamp + 90000);
            caller.wrapStake ( 1000000000);
            hevm.warp(block. timestamp + 90000);
            caller.wrapUnstake ( 1000000000);
        }
    
        assertTrue(false);
    }

Low-risk

LR-1. Reward is claimable even after program is ended

Status: Passed

Description

Regardless of the program being ended, users should be able to claim their rewards for their staked dafi tokens. This claim should not be bound by any type of time-related constraints. We checked whether this property would fail in any circumstance but it passed on all fuzzed inputs.

LR-2. Unit tests in the testing and main branch of repo at /test/v2.ts should pass

Status: Failed

Description

Unit tests are critical in proving the developer's expected intention and behavior of the working code hence the set of tests not passing entirely and partially in the testing repository code is slightly questionable.

Informatory issues and Optimizations

IR-1. Memory optimization by using uint8 in place of true/false

Description

Following the best practices and the Solidity design patterns guide and since the client code uses a good number of state variables to manage the switches for staking, unstaking and alike. It is therefore suggested by the auditing team that uint8 type variables can be replaced by the developer in place of true/false on multiple occasions to minimize the memory usage and reduce the code size as an optimization.

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.

INTRODUCTION

BlockApex (Auditor) was contracted by  VoirStudio  (Client) for the purpose of conducting a Smart Contract Audit/ Code Review. This document presents the findings of our analysis which started from  25th Feb 2022. 

Name: Unipilot-Farming-V2
Auditor: Kaif Ahmed | Muhammad Jarir Uddin
Platform: Ethereum/Solidity
Type of review: Manual Code Review | Automated Code Review
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/VoirStudio/unipilot-farming-v2
White paper/ Documentation: Not Provided
Document log: Initial Audit: 4th March 2022 (Complete)
Quality Control: 5th - 8th March 2022
Final Audit: 10th March 2022 (Complete)

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

Unipilot yield farming incentivizes Liquidity Providers to earn $PILOT tokens by staking their Unipilot LP tokens on whitelisted pools.

System Architecture

Unipilot yield farming has 1 main smart contract; UnipilotFarm.sol: which allows Liquidity Providers to earn $PILOT token, an $ALT token, or both by staking their Unipilot LP tokens. UnipilotFarm linearly distributes the $PILOT according to rewardPerBlock and rewardMultiplier.


Methodology & Scope

The codebase was audited in an iterative process. Fixes were applied on the way and updated contracts were examined for more bugs. We used a combination of static analysis tool (slither) and testing framework (hardhat) which indicated some of the critical bugs like reentrancy in the code. We also did manual reviews of the code to find logical bugs, code optimizations, solidity design patterns, code style and the bugs/ issues detected by automated tools.

AUDIT REPORT

Executive Summary

The analysis indicates that the contracts audited are working properly.

Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX, and Slither. All the flags raised were manually reviewed and re-tested. 

Our team found: 

# of issues Severity of the risk
3Critical Risk issue(s)
0High-Risk issue(s)
3Medium Risk issue(s)
3Low-Risk issue(s)
1Informatory issue(s)

Findings

#FindingsRiskStatus
1.First block reward is wrong in Only ALT and Dual Case.CriticalFixed
2.Consecutive change in reward type will disable the stakeLp() functionality.CriticalFixed
3.Miscalculation in pilot reward CriticalFixed
4.RewardToken validity should be checkMediumFixed
5.The UpdateRewardPerBlock() function will manipulate the last reward for every user.MediumFixed
6.Should allow Parameters to send Zero to reopen farmingMediumFixed
7.RewardToken Zero Address checkLowFixed
8.Necessary checks in updateFarmingLimit() functionLowFixed
9.Emergency exit for usersLowFixed
10.Should update Multiplier and RewardType while calling init function.InformatoryFixed

Critical-risk issues

  1. First block reward is wrong in Only ALT and Dual Case.

Description: 

If the reward type is changed from only Pilot to only ALT or only Pilot to Dual, the code updates the lastBlockReward variable first for Pilot then for ALT, calculating block difference as 2 instead of 0. Hence, the user gets triple reward for just the first block.

else {
            if (_rewardType == RewardType.Alt) {
                altState.lastRewardBlock = blockNumber;
                updateVaultState(_vault);
            } else {
                altState.lastRewardBlock = blockNumber;
                }
            }
        }
        emit RewardStatus(
            _vault,
            vaultInfo[_vault].reward,
            vaultInfo[_vault].reward = _rewardType,
            _altToken
        );

Remedy:

When updating the reward type lastRewardBlock should be set to the current block number.

else {
            if (_rewardType == RewardType.Alt) {
                altState.lastRewardBlock = blockNumber;
                vaultState.startBlock = blockNumber;
                updateVaultState(_vault);
            } else {
                altState.lastRewardBlock = blockNumber;
                }

Status: 

Fixed as per BlockApex recommendation. 

  1. Consecutive change in reward type will disable the stakeLp functionality.

Description: 

If the governance changes reward type from only Pilot to only Alt and then changes it again from only Alt to Dual, no user can stake their LPs.

Scenario: Before staking starts, if the last reward type is changed twice and is finally set to Dual, it causes the last reward block check, inside the stakeLp() function, to break, i.e., to not equal to start block, causing the getGlobalReward() function to be invoked, which in turn causes the mulDiv() error as the value of totalLpLocked never changed from zero.

 function getGlobalReward(
        address _vault,
        uint256 _blockDiff,
        uint256 _multiplier,
        uint256 _lastGlobalReward
    ) private view returns (uint256 _globalReward) {
        if (vaultInfo[_vault].totalLpLocked > 0) {
            _globalReward = FullMath.mulDiv(
                rewardPerBlock,
                _multiplier,
                1e18
            );

            _globalReward = FullMath
                .mulDiv(
                    _blockDiff.mul(_globalReward),
                    1e18,
                    vaultInfo[_vault].totalLpLocked
                )
                .add(_lastGlobalReward);
        } else {
            _globalReward = vaultInfo[_vault].globalReward;
        }
    }

Remedy:

If the totalLpLocked is zero,  the previous state of global reward should return the value of its previous state.  In case zero returns, the function should not calculate the new global reward.

function getGlobalReward(
        address _vault,
        uint256 _blockDiff,
        uint256 _multiplier,
        uint256 _lastGlobalReward
    ) private view returns (uint256 _globalReward) {
        if (vaultWhitelist[_vault]) {
            if (vaultInfo[_vault].totalLpLocked > 0) {
                _globalReward = FullMath.mulDiv(
                    rewardPerBlock,
                    _multiplier,
                    1e18
                );

                _globalReward = FullMath
                    .mulDiv(
                        _blockDiff.mul(_globalReward),
                        1e18,
                        vaultInfo[_vault].totalLpLocked
                    )
                    .add(_lastGlobalReward);
            } else {
                _globalReward = vaultInfo[_vault].globalReward;
            }
        } else {
            _globalReward = vaultInfo[_vault].globalReward;
        }
    }

Status:

Fixed as per BlockApex recommendation.

  1. Miscalculation in pilot reward.

Description: 

In an empty farm, if the reward type is changed from only Pilot to only ALT or only Pilot to Dual, the first user who stakes gets a reward of all the empty blocks since the reward type has been updated.

Remedy:

Modify the updateRewardType() function to check if there are no staked LPs in the farm, only then should the start block be updated to set as the current block number.

Status: 

Fixed as per BlockApex recommendation. 

High-risk issues

No issues were found

Medium-risk issues

  1. RewardToken validity should be checked.

Description: 

if (!vaultWhitelist[_vaultt[i]] && vaultState.totalLpLocked == 0) {
  insertVault(_vaultt[i], _multipliert [i]);
} else {
  require(!vaultWhitelist[_vault t [i]], "AI");
  if (vaultstate reward == RewardType.Dual) {
    vaultState. LastRewardBlock = blockNum;
    vaultaltState. lastRewardBlock = blockNum;
  } else if (vaultstate reward = RewardType.Alt) {
      vaultAltState. lastRewardBlock = blockNum;
  } else {
      vaultState. LastRewardBlock = blockNum;
  }
}

Remedy:

initializer() and UpdateRewardType() functions should check in the vault contract if the token actually exists by calling the balanceOf() function.

if (!vaultWhitelist[_vault [i]] && vaultstate.totalLpLocked == 0) {
if (
  _rewardType 1 (i) = RewardType. Alt |||
  _rewardType 1 [i] == RewardType.Dual
{
require(
  IERC20(_rewardToken ^ [i]).balanceof(address(this)) > 0,
  "NEB"
);
  vaultAltstate, multiplier = _multipliert[i];
  vaultAltstate.startBlock = blockNum;
  vaultAltState. LastRewardBlock = blockNum;
  vaultAltState. rewardToken = _rewardTokent [i];
}

Status:

Fixed as per BlockApex recommendation.

  1. The UpdateRewardPerBlock function will manipulate the last reward for every user.

Description: 

The event in updateRewardPerBlock() function emits the old reward value as well as updating and sending the new reward value, the problem is that this event emits at the start of the function which manipulates the last block reward for every user in the vault.

function updateRewardPerBlock(uint256 _value)
        external
        override
        onlyGovernance
    {
        emit RewardPerBlock(rewardPerBlock, rewardPerBlock = _value);
        require(_value > 0, "IV");

Remedy:

Event should be fired after the calculation at the end of the function. 

function updateRewardPerBlock(uint256 _value)
        external
        override
        onlyGovernance
    {
        require(_value > 0, "IV");
        address[] memory vaults = vaultListed();
        for (uint256 i = 0; i < vaults.length; i++) {
            if (vaultWhitelist[vaults[i]]) {
                if (vaultInfo[vaults[i]].totalLpLocked != 0) {
                    if (vaultInfo[vaults[i]].reward == RewardType.Dual) {
                        updateVaultState(vaults[i]);
                        updateAltState(vaults[i]);
                    } else if (vaultInfo[vaults[i]].reward == RewardType.Alt) {
                        updateAltState(vaults[i]);
                    } else {
                        updateVaultState(vaults[i]);
                    }
                }
            }
        }
        emit RewardPerBlock(rewardPerBlock, rewardPerBlock = _value);
    }

Status:

Fixed as per BlockApex recommendation

  1. Should allow Parameters to send Zero to reopen farming

Description: 

There is a check in updateFarmingLimit()  function which does not allow sending zero value in parameter, but it contradicts with the functionality. If the gov sets the farming limit to a specific block and they want to reopen or update the limit they have to send zero value to the updateFarmingLimit() function which will not work if zero value check is placed.

function updateFarmingLimit(uint256 _blockNumber)
        external
        override
        onlyGovernance
    {
       require(_blockNumber > 0, "IV");
        emit UpdateFarmingLimit(
            farmingGrowthBlockLimit,
            farmingGrowthBlockLimit = _blockNumber
        );
        updateLastBlock();
    }

Remedy:

Zero value check should be removed from the function.

function updateFarmingLimit(uint256 _blockNumber)
        external
        override
        onlyGovernance
    {
        emit UpdateFarmingLimit(
            farmingGrowthBlockLimit,
            farmingGrowthBlockLimit = _blockNumber
        );
        updateLastBlock();
    }

Status:

Fixed as per BlockApex recommendation

Low-risk issues

  1. RewardToken validity check

Description: 

No zero address check placed for RewardToken while calling the initializer() function.

 function initializer(
        address[] calldata _vault,
        uint256[] calldata _multiplier,
        RewardType[] calldata _rewardType,
        address[] calldata _rewardToken
    ) external override onlyGovernance {
        require(_vault.length == _multiplier.length,"LNS");
        uint256 blockNum = block.number;
        for (uint256 i = 0; i < _vault.length; i++) {

Remedy:

Zero address check should be Placed.

Status:

Fixed as per BlockApex recommendation

  1. Necessary checks in updateFarmingLimit() function.

Description: 

If the updateFarmingLimit() function is called with the same value of the block number in which it is going to be executed (or the past block number), the tx will be mined but it will not limit the farming as expected.

function updateFarmingLimit(uint256 _blockNumber)
        external
        override
        onlyGovernance
    {
        emit UpdateFarmingLimit(
            farmingGrowthBlockLimit,
            farmingGrowthBlockLimit = _blockNumber
        );
        updateLastBlock();
    }

Remedy:

A check should be placed in the updateFarmingLimit() function to ensure that block number never equals to current block or past block.

Status:

Fixed as per BlockApex recommendation

  1. Emergency exit for users.

Description: 

In event of any mishap with $ALT or $PILOT reward, a user won't be able to withdraw their LP funds. A user calls the unstakeLp()  function, the contract will throw the “Insufficient balance” error.

Remedy:

Contract should have emergency withdraw()  function to withdraw user’s staked LPs 

Status:

Fixed as per BlockApex recommendation

Informatory issues and Optimization

  1. Should update Multiplier and RewardType while calling init  function.

Description: 

It's extra work for the governance, for the first time if they want to set a vault for only ALT reward they have to call 3 different functions, this work can be done by calling only one function. 

function initializer(
        address[] calldata _vault,
        uint256[] calldata _multiplier
    ) external override onlyGovernance {
        require(_vault.length == _multiplier.length,"LNS");
        uint256 blockNum = block.number;
        for (uint256 i = 0; i < _vault.length; i++) {
            VaultInfo storage vaultState = vaultInfo[_vault[i]];
            AltInfo storage vaultAltState = vaultAltInfo[_vault[i]];

Remedy:

RewardType and RewardToken should be set by Calling Initializer() function and later it can be handled by individual functions.

function initializer(
        address[] calldata _vault,
        uint256[] calldata _multiplier,
        RewardType[] calldata _rewardType,
        address[] calldata _rewardToken
    ) external override onlyGovernance {

Status:

Fixed as per BlockApex recommendation

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.

INTRODUCTION

BlockApex (Auditor) was contracted by Voirstudio (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which took place on   _9th November 2021___ . 

Name: Unipilot Farming
Auditor: Moazzam Arif | Muhammad Jarir ud din
Platform: Ethereum/Solidity
Type of review: Staking and Farming
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/VoirStudio/unipilot-farming-contract/tree/dev
White paper/ Documentation: UnipilotFarm Contract Checklist
Document log: Initial audit completed on 12th November 2021, Final audit completed on 15th November 2021

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

Unipilot yield farming incentivizes LPs to earn $PILOT by staking their Unipilot NFT of whitelisted pools.

System Architecture

Unipilot yield farming has 1 main smart contract.

UnipilotFarm.sol: Smart contract which allows LPs to earn $PILOT by staking their Unipilot NFTs. UnipilotFarm linearly distributes the $PILOT according to rewardPerBlock and rewardMultiplier

Methodology & Scope

The codebase was audited in an incremental way. Fixes were applied on the way and were re-audited. We used a static analysis tool (slither) which indicated the reentrancy bug in the code. We did manual reviews on the code to find logical bugs and the bugs reported by the automated tools.

AUDIT REPORT

Executive Summary

The analysis indicates that the contracts audited are working properly.

Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX and Slither. All the flags raised were manually reviewed and re-tested. 

Our team found

# of issues Severity of the risk
1Critical Risk issue(s)
1High Risk issue(s)
2Medium Risk issue(s)
3Low Risk issue(s)
0Informatory issue(s)

FINDINGS

Critical-risk issues

  1. DOS in deposit

Description
The smart contract maintains a global reward per LP share as the pool's globalReward. It is calculated as the following formula:

globalReward = FullMath.mulDiv(blockDifference.mul(temp), 
1e18,poolInfo[pool].totalLockedLiquidity).add(_globalReward)

When the last user in the pool withdraws his Unipilot NFT, totalLockedLiquidity is
set to 0. When the next user tries to deposit in the same pool, the contract throws an error (div by 0).

Remedy:

This edge case should be handled.

Dev’s Response:

They acknowledged and fixed by resetting the pool’s variables when the last users removes NFT

Status:

Fixed and Verified

High-risk issues

  1. Reentrancy when distributing Alt token Rewards

Description:
Unipilot also helps other tokens (specially new tokens) to gain traction by rewarding LPs with this Alt token. So the LPs will be rewarded $PILOT and $ALT_TOKEN. Now when the user claims his reward, Unipilot transfers the shares of $ALT_TOKEN but updates the user reward debt after transferring. Now if the $ALT_TOKEN is ERC777, reentrancy is possible. Although it uses IERC20, but it will work with ERC777 tokens

IERC20(poolAltState.altToken).safeTransfer(userInfo[_tokenId].user, altReward);
poolAltState.lastRewardBlock = block.number;

Remedy:

Update all state variables before transfer. And also use ReentrancyGuard.

Dev’s Response:

We already have used nonReentrant modifier in the new commit

Status

Fixed and verified

Note:
It is good to communicate the changes earlier, so the auditor know beforehand the commit he is auditing

Medium-risk issues

  1. Wrong assumption in require statement

Description:
In depositNft the smart contract assumes that the user shares of liquidity should be less than totalLiquidity in the pool and shares of liquidity should be greater than zero. But in the following line it uses or( || ) instead of and (&&) operator

 require(totalLiquidity >= liquidity || liquidity > 0, "IL");

Remedy:

AND (&&) should be used.

Dev’s Response

Acknowledged

Status:

Not Fixed yet

  1. Unfair Reward distribution when pilotPerBlock is changed

Description:

There is a global variable pilotPerBlock which is used to calculate the $PILOT reward and act as a multiplier. This variable can be updated via governance. But When this variable is updated, it multiplies with the whole duration of staking.

Remedy:

Implement a mechanism like updating the rewardMultiplier of the pool by looping on all pools. Or just remove pilotPerBlock and just use rewardMultiplier to adjust pool rewards

Dev’s Response:

Acknowledged and will loop on all pools

Status:

Fixed and Verified

Low-risk issues

1. Missing Event in migrateFunds

2. Wrong event in updateUnipilot

3. Remove unused imports like LiquidityAmount.sol

Informatory issues and Optimization

No issues were found

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.

INTRODUCTION

BlockApex (Auditor) was contracted by KaliCo LLC_ (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which took place from 20th of December 2021 . 

Name: LexDAO/KaliDAO
Auditor: Moazzam Arif | Kaif Ahmed
Platform: Ethereum/Solidity
Type of review: Manual code review / Behavioral testing
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/lexDAO/Kali/tree/299a23a084b8f826f591b30725a3d8b512520ec7
White paper/ Documentation: https://github.com/lexDAO/Kali
Document log Initial Audit: 30th December 2021 (complete) Final Audit: (pending)

Scope 

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows: 

Code reviewFunctional review
ReentrancyUnchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering
Dependence
Repository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopComplexity of codeOperation Trails & Event Generation

Project Overview 

Kali is a protocol for on-chain organizations inspired by Compound and Moloch DAO Governance. Kali proposals are broken into a variety of types, such that each variance can have their own Governance settings, such as simple/super majority and Quorum requirements. 

System Architecture 

KaliDAO.sol 

KaliDAO is a Comp-style governance into a single contract, it supports extensions to add contracts as apps, for example crowdsale and redemption contracts. 

Kali supports hashing and amending docs from deployment and through proposals, providing a hook to wrap organizations into legal templates to rationalize membership rules and liabilities. 

KaliDAOtoken.sol 

KaliDAOtoken represent voting stakes, and can be launched as transferable or non-transferable, with such settings being updateable via PAUSE proposal. Voting weight can also be delegated, and such weight automatically updates upon token transfers from delegators, incorporating functionality from Comp-style tokens. 

Methodology & Scope 

Audit log:
In the first two days, we developed a deeper understanding of the DAO and its workings. We started by reviewing the two main contracts against common solidity flaws. After the reconnaissance phase we wrote unit-test cases to ensure that the functions are performing their intended behavior. Then we began with the line-by-line ma

AUDIT REPORT

Executive Summary

The analysis indicates that the contracts audited are working properly.

After the initial audit, the client was provided with the initial audit report, and the issues reported were discussed. After the fixes had been made, our team performed a re-audit of the codebase. No further issues were found. The contracts were separately reviewed by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX and Slither. All the flags raised were manually reviewed and re-tested.

Our team found:

# of issuesSeverity of the risk
Critical Risk issue(s)
High Risk issue(s)
Medium Risk issue(s)
Low Risk issue(s)
Informatory issue(s)

INITIAL FINDINGS

S#Findings RiskStatus
CR-1. “Arbitrary call” proposals can lead to unauthorized transfers.Critical-riskAcknowledged
CR-2. New “Proposals” should not be processed before processing previous “Proposals”.Critical-riskResolved
HR-1. A “SUPERMAJORITY” proposal can be triggered without casting any votes.High-riskResolved
HR-2.  Whitelisting can be bypassed in extension.High-risk Acknowledged
HR-3.Any malicious user can withdraw “purchaseTokens” and drain the
whole contract
.
High-riskResolved
IR-1.Upper and lower bound on Governance params.Informatory
issues
Acknowledged
IR-2.“Pause” proposal should escape previous proposals.Informatory
issues
Acknowledged
IR-3.Bad error on proposal: “PROCESSED” for non-existent proposals.Informatory
issues
Acknowledged
IR-4.Bad error on voting: “VOTING_ENDED” for non-existent proposals.Informatory
issues
Acknowledged

Update on initial findings

Critical-risk issues

CR-1. “Arbitrary call” proposals can lead to unauthorized transfers 

Contract : KaliDAO.sol 

it("proposal type call , using arbitrary calls to call transfer and transferFrom function of kalidao token ", async function () {

    // hex data for payload
    const tranferFromCall = lexdao.interface.encodeFunctionData("transferFrom", [owner.address,addr3.address, 25]);
    
    // any approved users balance for lexDAO
    await lexdao.approve(lexdao.address , 1000000000);

    const ownerPreBal = await lexdao.balanceOf(owner.address);

    // proposalType.CALL, accounts = [lexdaoAddress], value = 0 ether, call = transferFromCall data
    await lexdao.propose(2, "TEST", [lexdao.address] , [0], [tranferFromCall]);
    await lexdao.vote(0,true); // votecount check
    await forwordTime(40); // vote ended, ignore typo :)
    await lexdao.processProposal(0); // process proposal
    
    // after balances
    const add3AftBal = await lexdao.balanceOf(addr3.address);
    const ownerAftBal = await lexdao.balanceOf(owner.address);

    // balances tranferred
    expect(add3AftBal.toNumber()).not.eq(0); // tranferFrom call executed
    expect(ownerAftBal.toNumber()).lessThanOrEqual(ownerPreBal.toNumber());
    
});

})

Exploit Scenario 

Any malicious user can submit the proposal type call, vote type simple majority and payload has a transferFrom transactions as we created payload on above test. Attackers can transfer any amount from any user if the user has approved his funds to the LexDao contract. 

Remedy 

1. BlackList specific account addresses and calls. 

2. Implement validation of transaction data. (this might create centralization) 

Developer's Response 

“We believe this issue is avoided by discouraging users from approving token pulls to the `KaliDAO.sol` contract. Instead, payments to DAOs will be through direct transfers or by the use of extension contracts which are authorized to make such token pulls. It is our preference to avoid limiting the kinds of calls that can be made through these kinds of proposals or adding centralization factors” 

Auditor's Response 

Status : Acknowledged. 

CR-2. New “Proposals” should not be processed before processing previous Proposals 

Contract : KaliDAO.sol 

Function : processProposal() 

// skip previous proposal processing requirement in case of escape hatch
        if (prop.proposalType != ProposalType.ESCAPE) {
            // allow underflow in this case to permit first proposal
            unchecked {
                require(proposals[proposal - 1].creationTime == 0, 'PREV_NOT_PROCESSED');
            }
        }

Exploit Scenario 

As stated in the piece of code mentioned above, it is very clear that if any previous proposal is pending, no new proposal will be processed. However, this check can be easily by-passed if a non-member adds a proposal and then sponsorProposal is called after that. This way, a new proposal will be processed. 

it("Process proposal without processing previous proposals", async function () {

    //proposal 0
    await lexdao.propose(0, "TEST", [owner.address] , [1], [0x00]);
    //proposal 1
    await lexdao.propose(0, "TEST", [owner.address] , [1], [0x00]);
    //proposal 2 by non-menber
    await lexdao.connect(addr3).propose(0, "TEST", [owner.address] , [1], [0x00]);
    //sponsor by member
    await lexdao.sponsorProposal(2);
    await forwordTime(40);
    try {
        //process Proposal without process previous proposals
        await lexdao.processProposal(3);
    } catch (error) {
        console.log(error);
    }
});

Remedy 

Do not create a “new” proposal when sponsorProposal() is called. 

Developer's Response 

“We have made a fix by tracking a new global state variable, currentSponsoredProposal and updating it upon every sponsored proposal. This value gets appended to the Proposal struct and is checked on processing, like so: 

if (proposals[prop.prevProposal].creationTime != 0) revert PrevNotProcessed(); 

Auditor's Response 

Status : Resolved. 

High-risk issues

HR-1. A “SUPERMAJORITY” proposal can be triggered without casting any votes 

Contract : KaliDAO.sol 

Function : _countVotes() 

function _countVotes(
        VoteType voteType,
        uint256 yesVotes,
        uint256 noVotes
    ) internal view virtual returns (bool didProposalPass) {
        // rule out any failed quorums
        if (voteType == VoteType.SIMPLE_MAJORITY_QUORUM_REQUIRED || voteType == VoteType.SUPERMAJORITY_QUORUM_REQUIRED) {
            uint256 minVotes = (totalSupply * quorum) / 100;
            
            // this is safe from overflow because `yesVotes` and `noVotes` are capped by `totalSupply`
            // which is checked for overflow in `KaliDAOtoken` contract
            unchecked {
                uint256 votes = yesVotes + noVotes;

                if (votes < minVotes) return false;
            }
        }
        
        // simple majority
        if (voteType == VoteType.SIMPLE_MAJORITY || voteType == VoteType.SIMPLE_MAJORITY_QUORUM_REQUIRED) {
            if (yesVotes > noVotes) return true;
        // super majority
        } else {
            // example: 7 yes, 2 no, supermajority = 66
            // ((7+2) * 66) / 100 = 5.94; 7 yes will pass
            uint256 minYes = ((yesVotes + noVotes) * supermajority) / 100;

            if (yesVotes >= minYes) return true;
        }
    }

Exploit Scenario 

Let's consider the code above; The else() statement : if yesVotes and noVotes are zero and supermajority is set to 66, the resulting minYes will ultimately be zero as well. Now let’s see the last if() inside the else( ) condition. Both yesVotes and minYes are zero, so the condition will return true. If any malicious user adds a proposal of mint/burn, they just have to wait to process the proposal because that proposal does not need any votes. 

Remedy 

Remove the equality sign and ensure that the voting period is long enough so that the users are able to vote on every proposal. This will make sure that the yesVotes required to process a proposal is greater than 0 and can get votes from other users as well. 

Developer's Response 

We have a made a fix to this issue by putting in a return check at the top of the “_countVotes” internal function that fails a proposal if nobody participated, like so: “if (yesVotes == 0 && noVotes == 0)” 
return false;   

Auditor's Response 

Status : Resolved. 

HR-2. Whitelisting can be bypassed in extension 

Contract : KaliDAOcrowdsale.sol 

         if (sale.listId != 0) require(whitelistManager.whitelistedAccounts(sale.listId, account), 
            'NOT_WHITELISTED');

Exploit Scenario 

User can bypass the whitelisting check by providing listId “0”. 

(Note: This might be a false assumption because we didn't properly recon the latest codebase at this point) 

Remedy 

A contract should have proper checks that restrict users to set extension with “0” listId. Developer's Response 

In the crowdsale extension contract, we purposefully allow users to set a null value for whitelist ID in order to allow for unrestricted token sales. 

Auditor's Response 

Status : Acknowledged. 

HR-3. Any malicious user can withdraw “purchaseToken” and drain the whole contract 

Contract : KaliDAOcrowdsale.sol 

it("Bypassing whitelisting can lead to unautherized transfers " , async function() {
    // Data for crowdsale Extension
    const data = abiCoder.encode(
        ["uint256" , "address" , "uint8" , "uint96" , "uint32"] , 
        [0 , token.address , 1, BigNumber("100000").toString() , BigNumber("1640838044").toString()]);
    //transfering funds to address 3
    await token.transfer(addr3.address , BigNumber("100").toString());
    //checking balance before
    console.log(await token.balanceOf(addr3.address));
    console.log(await token.balanceOf(addr4.address));
    //approving funds by address 3 to crowdsale contract
    await token.connect(addr3).approve(crowdsale.address , BigNumber("100").toString());
    //set extension using address 4
    await crowdsale.connect(addr4).setExtension(data);
    //call extension using address 4
    await crowdsale.connect(addr4).callExtension(addr3.address, BigNumber("100").toString());
    //checking balance after
    console.log(await token.balanceOf(addr3.address));
    console.log(await token.balanceOf(addr4.address));

})

Exploit Scenario 

Consider that a user can set an extension using the listId “0” (the check which can be easily bypassed). Now if any user has approved their funds to the crowdsale contract, any malicious user can simply call the setExtension() using the token address in his data and then call callExtension() using address 3 (used in the example above) in the parameters which will ultimately withdraw all the funds user have approved to the crowdsale contract. 

Remedy 

Whitelisting should be handled properly like funds should be transferred to a whitelisted address instead of msg.sender. 

Developer's Response 

Resolved the issue by limiting users to call the extension directly from crowdsale and deducting amounts form msg.sender instead of any address. 

Auditor's Response 

Status : Resolved. 

Informatory issues and Optimization 

IR-1. Upper and lower bound on Governance params 

Contract : KaliDAO.sol 

Description 

init function has proper upper and lower bounds but if the user set values by Governance, it has no lower and upper bound check. Unchecked math assumes these bounds. 

Remedy 

A proper upper and lower bounds check should be placed while changing values by Governance. 

Developer's Response 

We provide reversion checks on governance params in the “propose()” function to ensure that proposals to amend “votingPeriod”, “quorum”, “supermajority” and proposal types are kept within expected bounds. 

Auditor's Response 

Status : Acknowledged. 

IR-2. “Pause” proposal should escape previous proposals 

Contract : KaliDAO.sol 

Description 

To pause the contract, a proposal is submitted. Most of the time a pause is needed in emergencies. To timely execute a pause proposal it should not wait for previous proposals. (We have seen mishaps with $COMP in the past, where their proposal to fallback takes days). 

Remedy 

There should be a check in processProposal() that if the proposal type is “pause” it can execute right away. 

Developer's Response 

It is our preference to maintain a voting period for PAUSE proposal types in order to accommodate the use case of gradual decentralization by some DAOs. For example, there are non-emergency situations where a DAO might deploy with a closed founder group, grow its membership through proposals, but then want to vote and reach consensus on making membership tokens transferable, and therefore, open to the public. 

Auditor's Response 

Status : Acknowledged. 

IR-3. Bad error on proposal: “PROCESSED” for non-existent proposals Contract : KaliDAO.sol 

 function processProposal(uint256 proposal) public nonReentrant virtual returns (
        bool didProposalPass, bytes[] memory results
    ) {
        Proposal storage prop = proposals[proposal];

        require(prop.creationTime != 0, 'PROCESSED');

Description 

If a user tries to process a non-existent proposal, the above code won’t let him process because proposal creation time is 0 but the user will get the “PROCESSED” error. This should be handled properly by separately checking if the proposal actually exists or not. 

Remedy 

“Proposal does not exist” should be displayed by adding a check using require. Developer's Response 

We have provided a clarified reversion message, “Processed()” -> “NotCurrentProposal()”. We otherwise would like to avoid additional checks to optimize for gas efficiency. 

Auditor's Response 

Status : Acknowledged. 

IR-4. Bad error on voting: “VOTING_ENDED” for non-existent proposals Contract :

   // this is safe from overflow because `votingPeriod` is capped so it will not combine
        // with unix time to exceed 'type(uint256).max'
        unchecked {
            require(block.timestamp <= prop.creationTime + votingPeriod, 'VOTING_ENDED');
        }

KaliDAO.sol

Description 

If a user tries to vote on a non-existent proposal, the above code won't let him vote because proposal creationTime + votingPeriod will be less than block.timestamp but the user will get “VOTING_ENDED” error. This should be handled properly by separately checking if the proposal actually exists or not. 

Remedy 

“Proposal does not exist” should be displayed by adding a check using require. 

Developer's Response 

We have provided a clarified reversion message, “VotingEnded()” -> “NotVoteable()”. We otherwise would like to avoid additional checks to optimize for gas efficiency. 

Auditor's Response 

Status : Acknowledged. 

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. 

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks. 

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.

INTRODUCTION

BlockApex (Auditor) was contracted by Sonar(Client) for the purpose of conducting a Smart Contract Audit/Code Review.  This document presents the findings of our analysis which took place on 8th September 2021. 

Name: Sonar BSC-ETH bridge
Auditor: Moazzam Arif | Kaif Ahmed
Platform: Ethereum/Solidity
Type of review: Bridge
Methods: Architecture Review, Functional Testing, Manual Review
Git repository: https://github.com/sonarplatform/eth_bridge/tree/4c086d33fb5cb7a04a2192d6566c7905ea3b1074
White paper/ Documentation: Not provided
Document log: Initial Audit (08-09-2021) Final Audit (pending)

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

This is a POA bridge application that provides eth to bsc and bsc to eth bridging. It allows its native tokens to be accessed on both ETH and BSC blockchains.

System Design

Steps to burn and mint (BSC => ETH):

1. User calls the burn method on BSC bridge (BSC blockchain)

2. With the burn method, tokens are transferred to the admin account 

3. Transfer event is emitted on successful burn

4. Relayer listens the event and call the mint method on ETH bridge 

5. With the mint method, the tokens are transferred to the user on the ethereum blockchain

For ETH => BSC similar steps are followed. 

Test Cases

No test cases were provided, we did our own simulations. 

AUDIT REPORT

Executive Summary

The analysis indicates that the contracts audited are insecure..

Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After a thorough and rigorous process of manual testing, all the flags raised were iteratively reviewed and tested.

Our team found:

# of issues Severity of the risk
2Critical Risk issue(s)
0High Risk issue(s)
1Medium Risk issue(s)
1Low Risk issue(s)
1Informatory issue(s)

 

Findings

Critical-risk issues

1. Anyone can lock funds of other users

If user1 wants to transfer tokens from BSC to ETH, user1 will first approve funds to the BSC bridge. Now that the user has approved his funds to the bridge contract, anyone else can call the burn method passing the address of user1, the funds of user1 have been locked by someone else. 

Remedy:
Burn function contains following logic 

token.transferFrom(to, address(this), AmountwithFees);

The parameter contains “to” but should contain “msg.sender”

2. Server API to mint the tokens

There is an api written in the server to mint tokens. It does not verify that the burn has been called or not. The users can mint without burning using that api.

Remedy:
User should submit proof of burn before calling the minting API

High-risk issues

No high-risk issues were found.

Medium-risk issues

1. Locking/Unlocking both side
The contract has locking and unlocking functionality on both sides. For minting it is assumed that the admin wallet should have enough token balance. 


Attack scenario:

1. User 1 locks 1000 tokens on BSC bridge 

2. There is not enough token balance equivalent to be given to user1 on ETH bridge

require(  adminBal > amount, "insuffient funds in admin account.");

3. Now user1 can not unlock on ETH bridge

a. He will have to unlock his token again to get his funds back from BSC bridge (extra gas, because the function is not performing the intended behavior)

b. Unlocking of tokens on the same bridge is inconvenient from user’s perspective

Remedy:
Usually the lock/unlock strategy is used on one bridge and mint/burn strategy is used on the other bridge.

Low-risk issues

1. Centralization Risk
Admin wallet is a single wallet and there is high risk of centralization if the private key gets compromised. 

 require(admin == msg.sender, "only admin can call this");


Remedy:
Assignment of privileged roles to multi-signature wallets to prevent single point of failure due to the private key.

Informatory issues

1. Lock pragma versions

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.
Remedy:

Lock the pragma version and also consider known bugs (https://github.com/ethereum/solidity/releases) for the compiler version that is chosen. Please refer to this doc for more details.

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts. 


INTRODUCTION

BlockApex (Auditor) was contracted by    SONAR    (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which took place on   28th September 2021  . 

Name: Sonar BSC-ETH bridge v2
Auditor: Moazzam Arif | Kaif Ahmed
Platform: Ethereum/Solidity
Type of review: Bridge
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/XORD-one/sonar-bridge-contract/tree/17cb255444153945b66c5da8672f2ca06ec91efd
White paper/ Documentation: Not provided
Document log: Bridge v1 Audit (08-09-2021) | Bridge v2 Audit (28-09-2021)*

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

This is a POA bridge application that provides ETH to BSC and BSC to ETH bridging. It allows its native tokens to be accessed on both ETH and BSC blockchains.

System Architecture

Steps to burn and mint (BSC => ETH):

1. User calls the burn method on BSC bridge (BSC blockchain)

2. With the burn method, tokens are transferred to the admin account 

3. Transfer event is emitted on successful burn

4. Relayer listens the event and call the mint method on ETH bridge 

5. With the mint method, the tokens are transferred to the user on the ethereum blockchain

For ETH => BSC similar steps are followed. 

Methodology & Scope

Smart contract for the native token (PING) was provided. As $PING is similar to $RFI and $SAFEMOON. So the swapping and fee distributions were primarily focused during this security assessment (as they had already been audited by Certik). Only minting and burning functionality of $ePING (ethereum representation of $PING) was considered in scope. Manual Review and Static Analysis tools were used to produce this report.

AUDIT REPORT

Executive Summary

The analysis indicates that some of the functionalities in the contracts audited are not working properly.


Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX and Slither. All the flags raised were manually reviewed and re-tested. 

Our team found: 

# of issues Severity of the risk
0Critical Risk issue(s)
1High Risk issue(s)
0Medium Risk issue(s)
1Low Risk issue(s)
5Informatory issue(s)

Findings

Critical-risk issues

No issues were found

High-risk issues

1. No Minting and Burning in ePING
File:  ePING.sol
_tTotal represents the totalSupply. Initially on the second chain _tTotal is set to 0 in the constructor. _tTotal is used to calculate the currentRate and currentRate is used while transferring and burning. Setting _tTotal to zero gives math errors. While minting _rTotal is so high (~uint(0 => ~MAX value of uint256)). Adding any value to _rTotal will cause underflow.
NOTE: Please carefully set the _rTotal. Also while minting do not change the currentRate, as it will be reflected in other users’ balances

Medium-risk issues

No issues were found.

Low-risk issues

1. GasFee Griefing Attack
Assuming that the relayer server (owned by the client) pays the gasFee to mint/unlock tokens on the alternate bridge. As there is no min_amount limit on bridging tokens. Consider the following Attack Scenario
Attack Scenario:
1. User deposits 0/1 wei amount of token on BscBridge.
2. Relayer server picks-up the deposit event and calls the mint function on etheremBridge.
3. User receives 0/1 wei tokens. And Relayer pays the gas.
4. As gas fees are high, we can deduce that 1 wei tokens < gasFee (paid by relayer).
Remedy:
1. Charge gas fee from the user. While bridging the tokens, mint amount - gasFeeEquivalent on alternate chain.
2. Put a minimum threshold on the amount of tokens to be bridged.

Informatory issues and Optimization

1. transactionID is used while depositing into the bridges. transactionID is not verified in the smart contracts. It is used when the deposit event is emitted. We advise you to validate transactionID at the backend (relayer servers).

2. Destination chainId should be used while depositing. This will improve user experience if multiple chains are supported for bridging in future.

3. Remove console.log from smart contract.

4. IToken.sol has a wrong implementation of interface. Use Openzeppelin’s ERC20 interface.

5. Fees are deducted on every transfer. Please make sure that the bridge addresses are excluded from the fees.

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.


INTRODUCTION

BlockApex (Auditor) was contracted by __PhoenixDAO__ (Client) for the purpose of conducting a Smart Contract Audit/Code Review.  This document presents the findings of our analysis which took place on   ___28th October 2021____. 

Name: PhoenixDAO
Auditor: Muhammad Jariruddin | Kaif Ahmed
Platform: Ethereum/Solidity
Type of review: LP staking | DAO
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: https://github.com/Musfirazia/PhoenixStaking/tree/3aec9851bd517e9e8da4d72193367cfa9f6cc863
White paper/ Documentation: Not provided
Document log: Initial Audit published (4th October 2021) Final Audit published (3rd November 2021)

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

PhoenixDAO

File: DaoSmartContract.sol

DAO smart contracts used to maintain onchain proposals. Voting is maintained off-chain. To avoid spams, proposers must have to deposit collateral in $PHNX.

Steps to successful proposal:

1-User submits a proposal and add collateral (Note: collateral is not deducted at at this point)

2-Off-chain voting happens

3-Admins change the status of proposals to Upvote on smart contract.

4-Collateral amount is deducted from the proposer's account.

5-When the status reaches completed, proposer’s collateral is returned back.

Phoenix Spot Staking

File: DaoStakeContract.sol

Users can earn spot staking rewards by depositing $PHNX for set period of time. If a user unstakes before the set duration, his portion (according to a formula) of staking amount will be burnt. Regardless, the user will get the full rewards at the time of staking.

Phoenix LP staking

File: phxStake.sol

Users can earn rewards by staking their LP tokens. This staking style is similar to Pancakeswap’s MasterChef.

AUDIT REPORT

Executive Summary

The final analysis indicates that the contracts audited are well-secured. Most of the issues reported in the initial review have been fixed by the client. The contracts were reviewed again in order to ensure maximum security.


Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX and Slither. 

 Our team found: 

# of issues Severity of the risk
2Critical Risk issue(s)
0High Risk issue(s)
2Medium Risk issue(s)
0Low Risk issue(s)
1Informatory issue(s)

FINDINGS

Critical-risk issues

File: DaoSmartContract.sol

1. User can withdraw the amount even if the actual deposit didn’t happen

Attack Scenario:
User submit a proposal with collateral amount X. As the collateral is deducted when the proposal status is changed to UpVotes by admins. User can withdraw collateral amount of tokens(even if the tokens are not actually deducted). Only checks in place are:

require(!proposalList[proposalId].isClaimed, "Collateral Already Claimed");
require(proposalList[proposalId].proposer == sender, "Only Owner of Proposal can withdraw");

Though this function requires to be called by admins, we suggest that please verify at the backend that the user is eligible to withdraw or not.


Remedy:
Verify that the user has his collateral deducted by just adding a flag in proposal struct (collateralDeposited). 

Dev’s response:
It is only possible when the user has a signature and the user cannot have a signature. Verified at the backend.

2. User can withdraw even if the collateral is returned when the status of proposal is completed

Attack Scenario:
When the admin changes the status of the  proposal to completed, the contract automatically transfers the collateral back to the proposer. Still there are no checks placed and the collateral is returned back.

Remedy:
Add a flag (collateralReturned) in the proposal struct and verify the flag in withdraw.    

Dev’s response:
It is only possible when the user has a signature and the user cannot have a signature. Verified at the backend.

High-risk issues

No high-risk issues were found.

Medium-risk issues

1. Possible DOS attack in unstake

File: DaoStakeContract.sol

When users deposit to earn spot staking rewards. StakerData holds the staking information. StakerData needs stakeId to find staking information. stakeId  is generated as follows.

  bytes32 stakeId = keccak256(abi.encode(_timestamp, _beneficiary, _altQuantity));

When the  user unstakes, he has to provide stakeIds to unstake. Now the attacker can use the stakeIds of other users with the following constraint i.e., (sum of withdraw amount from all staking events should be less than attacker's staked amount). Attacker withdraws his amount, but now the original staker does not know his stakeIds. 

  stakerBalance[sender] = stakerBalance[sender].sub(withdrawAmount);

Note:
Although the attacker might have his stakes burnt, the other users will be unable to unstake (at least other users will a have hard time to find stakeIds).

Remedy:
We suggest verifying the stakeIds before processing.

Status:
Fixed.

2. LPTokens are transferred to $PHNX contract address

The Lp staking smart contract is similar to PCS MasterChef contract. When the  user deposits LPs,  updatePool method is called. This method transfers some of LP tokens to the $PHNX contract address. This will cause issues when the user tries to unstake their LPs because the contract wouldn’t have enough LPs.

Status:
Fixed.

Low-risk issues

No high-risk issues were found.

Informatory issues

1. Pid(pool id) arg is never used when a user withdraws in LP staking.  We suggest removing that. 

File: PhxStake.sol

Status:
Fixed.

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.


INTRODUCTION

BlockApex (Auditor) was contracted by    _Voirstudio__   (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which took place from   _19th August 2021__ . 

Audit Log

Entry #1
Date: 11th October 2021
In the first week, we developed a deeper understanding of the protocol and its workings. We started by reviewing the five main contracts against common solidity flaws and did a static analysis using Slither. In the second week, we wrote unit-test cases to ensure that the functions are performing their intended behaviour. In the 3rd week, we began with the line-by-line manual code review.

Note: We haven’t fuzzed the end-to-end smart contracts behaviour in this auditing round. So this is the version of the report with the issues that were found in all the steps we performed before the above mentioned date. We are now engaged again by Unipilot to further test and fuzz properties. A detailed analysis will be shared later in an updated version of the report which will be published later.

Entry #2
Date: 1st November 2021
We were tasked with an additional review of the protocol by Voirstudio to verify the mathematics and economics of Unipilot. We fuzzed the smart contracts to test properties of the protocol and found some issues which are reported below. We have tested and discussed a few bullets in the grey area regarding the economics of the protocol and worked out contingency plans around it.

Note: We worked closely with the developers and the fixes were incrementally applied. The team was very supportive and was open to suggestions and discussion. They even provided a few pointers as to where we would find potential vulnerabilities. 

Project Details

Name: Unipilot
Auditor: Moazzam Arif | Kaif Ahmed | Muhammad Jarir Uddin
Platform: Ethereum/Solidity
Type of review: Mathematics, Tokenomics, Liquidity Management, Index Fund
Methods: Architecture Review, Functional Testing, Computer-Aided Verification, Manual Review
Git repository: First review: https://github.com/VoirStudio/unipilot-protocol-contract-v2/tree/update-structure
Git repository: Second review: https://github.com/VoirStudio/unipilot-protocol-contract-v2/tree/update-getSharesAndAmounts
White paper/ Documentation: https://unipilot.medium.com/
Document log: 1st review: Initial Audit (19th August 2021) | Final Audit (11th October 2021) 2nd review: Initial Audit (20th October 2021) | Final Audit (26th October 2021)

Scope

The git-repository shared was checked for common code violations along with vulnerability-specific probing to detect major issues/vulnerabilities. Some specific checks are as follows:

Code reviewFunctional review
Reentrancy Unchecked external callBusiness Logics Review
Ownership TakeoverERC20 API violationFunctionality Checks
Timestamp DependenceUnchecked mathAccess Control & Authorization
Gas Limit and LoopsUnsafe type inferenceEscrow manipulation
DoS with (Unexpected) ThrowImplicit visibility levelToken Supply manipulation
DoS with Block Gas LimitDeployment ConsistencyAsset’s integrity
Transaction-Ordering DependenceRepository ConsistencyUser Balances manipulation
Style guide violationData ConsistencyKill-Switch Mechanism
Costly LoopOperation Trails & Event Generation

Project Overview

Unipilot is an auto-liquidity management protocol built on top of Uniswap v3. It simplifies the liquidity management by rebasing the liquidity of the pool, when prices get out of range.

To readjust the liquidity of the pool, Captains (independent nodes) are compensated for the gas fee plus some bonus in $PILOT.

It also has a governing token i.e., $PILOT. When a protocol earns a fee from Uniswap v3, users have the option to claim a fee in equivalent $PILOT (price is fetched from $FEE_TOKEN/$WETH -> $PILOT/$WETH price oracles).

System Architecture

The protocol is built to support multiple dexes (decentralized exchanges) for liquidity management. Currently it supports only Uniswap v3’s liquidity. In future, the protocol will support other decentralized exchanges like Sushiswap (Trident). So the architecture is designed to keep in mind the future releases.

The protocol has 5 main smart contracts and their dependent libraries.

Unipilot.sol: The smart contact is the entry point in the protocol. It allows users to deposit, withdraw and collect fees on liquidity. It mints an NFT to it’s users representing their individual shares.

V3Oracle.sol: It is a wrapper around Uniswap oracles. It also has helper functions to calculate TWAP (time-weighted average price) and other prices relevant data.

UniswapLiquidityManager.sol: The heart of the protocol that allows users to add, withdraw, collectFee and readjust the liquidity on Uniswap v3. This smart contract interacts with Uniswap v3Pool and v3Factory to add and remove liquidity. It maintains 2 positions on Uniswap i.e., base position and range position (leftover tokens are added as range orders).

UniStrategy.sol: The smart contract to fetch and process ticks’ data from Uniswap. It also decides the bandwidth of the ticks to supply liquidity (on the basis of governance).

ULMState.sol: This smart contract fetches the updated prices and tick ranges from Uniswap’s v3 pools.

Steps to Add Liquidity:

1-User will give approval of both tokens to Unipilot.sol

2-User will call the deposit method of Unipilot.sol 

3-Unipilot.sol’s deposit method will call the deposit method of UniswapLiquidityManager.sol and transfer both the tokens to UniswapLiquidityManager.sol

4-UniswapLiquditymanager.sol will add the liquidity on Uniswap. This will:
a. Mint an NFT denoting the user’s shares in the liquidity provided
b. 2-Alter the existing liquidity by increasing the user’s share

AUDIT REPORT

Executive Summary

In our first iteration, we found 1 critical-risk issue, 4 high-risk issues, 1 medium-risk, 1 low-risk issue and 1 informatory issue. All these issues were refactored and fixes have been made. A detailed report on the first review can be found here.

The second iteration was performed recently, with the updated codebase. We found a few more issues that were promptly fixed by the team. Our analysis indicates that the contracts are now secure.

Our team found:

# of issues Severity of the risk
1Critical Risk issue(s)
0High Risk issue(s)
3Medium Risk issue(s)
0Low Risk issue(s)
2Informatory issue(s)

 

FINDINGS

Critical-risk issues

1.  Ether balance of contract can be withdrawn

File: PeripheryPayments.sol

Description:
Unipilot conducted a testnet bug bounty. In that some users reported they were unable to collect their fees. By investigating that with the dev team, they founded that ERC20/WETH pool was misbehaving due to the following code snippet:

if (balanceWETH9 > 0) {
IWETH9(WETH).withdraw(balanceWETH9); // contract balance is transferred 
TransferHelper.safeTransferETH(recipient, balanceWETH9);} 

Remedy:
The dev team suggested to remove this method and pass proper amount to be withdrawn

Status:
Fixed

High-risk issues

No high-risk issues were found.

Medium-risk issues

1. Math Precision issues

File: UniswapLiquidityManger.sol & ULMState.sol

Description:
Unipilot uses 1e18 precision for calculating the Liquidity shares and feeGrowth when adding liquidity and fees. This causes some small amounts to be locked forever in the UniswapLiquiditymanager.

See the following code snippet:

       position.feeGrowthGlobal0 += FullMath.mulDiv(
collect0Base + collect0Range,
           1e18, // precision
   position.totalLiquidity
       );

           

Remedy:
We suggest using FixedPoint128.Q128 from Uniswap. We fuzzed against this precision and we got more precise results

Status:
Fixed

2. Safe cast and Safe Math

Description:

There are many instances in the contract which use unsafe math operations. This might lead to overflow/underflow. The contract also uses unsafe type casting.
See the following code snippet of unsafe cast:

 (int256 amount0Delta, int256 amount1Delta) = 
IUniswapV3Pool(b.poolAddress).swap(
     address(this),
     b.zeroForOne,
     int256(b.amountIn), // unsafe casting, use safeCast
     b.sqrtPriceLimitX96,
     abi.encode((SwapCallbackData({ token0: token0, token1: token1,
fee: fee })));

Remedy:
We suggest to use safe math and safe cast libraries
Status:
Fixes in progress

3. Economic Attacks

a. Price Oracle manipulation
File:
UniswapLiquidityManger.sol
Unipilot relies on TWAP when minting new $PILOT tokens, either in fees or in readjustment of the pool. Tokens are priced w.r.t to ether and then from etherAmount the pilotAmount is calculated.

Attack Scenario:
Consider users who want to get $PILOT in fees instead of underlying tokens. Users can create a pool with $WETH, do some swaps, and inflate the tokens against $WETH. Unipilot will get the etherAmount of tokens from the pool and convert ethers to pilot. Hence the user gets the $PILOT at a manipulated price.

Note:
To mitigate this attack, devs already have implemented whitelisting of pools when distributing fees. But they had not implemented whitelisting during readjustments.

Remedy:
Add whitelisting to readjustment

Status:
Fixed

b. Put a limit on tx.gasPrice

File: UniswapLiquidityManger.sol

Description:
When a user readjust the pool, his transaction fees is compensated plus a
Premium is given to the user in $PILOT.

b.gasUsed = tx.gasprice.mul(initialGas.sub(gasleft()));
 // tx.gasprice can be adjusted high enough to mint more $PILOT  
  b.pilotAmount = IOracle(_oracle).ethToAsset(PILOT,
  3000,b.gasUsed); 

Remedy:
Add a limit to tx.gasprice

Status:
Fixed

c. Probable Slippage Attacks
While readjusting the pool, the Unipilot swaps X amount of tokens to make the pool in range. The swap percent (how much should be swapped) and the slippage is hardcoded. This incentivizes sandwich attacks.

Remedy:
The swap amount and slippage should be configurable according to favorable conditions

Status:
Fixed

Low-risk issues 

No low-risk issues were found.

Informatory issues and Suggestions

1. Contracts should be made pausable in case of a mishap.

2. Contracts should have rescue funds methods.

DISCLAIMER

The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report. 

This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project. 

Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.

Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.

This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.


Stay in Touch

Drop your email to read the BlockApex newsletter and keep yourself updated around the clock.

    All rights reserved. Copyright 2020-21