Halborn Logo

icon

eth.nftfi - Collection Offer - NFTfi


Prepared by:

Halborn Logo

HALBORN

Last Updated 04/26/2024

Date of Engagement by: August 16th, 2022 - August 24th, 2022

Summary

100% of all REPORTED Findings have been addressed

All findings

2

Critical

0

High

0

Medium

0

Low

0

Informational

2


1. INTRODUCTION

NFTfi engaged Halborn to conduct a security audit on their smart contracts beginning on 2022-08-16 and ending on 2022-08-24. The security assessment was scoped to the smart contracts provided to the Halborn team.

2. AUDIT SUMMARY

The team at Halborn was provided two weeks for the engagement and assigned a full-time security engineer to audit the security of the smart contract. The security engineer is a blockchain and smart-contract security expert with advanced penetration testing, smart-contract hacking, and deep knowledge of multiple blockchain protocols.

The purpose of this audit is to:

    • Ensure that smart contract functions operate as intended

    • Identify potential security issues with the smart contracts

In summary, Halborn identified some security risks that were acknowledged by the NFTfi team.

3. TEST APPROACH & METHODOLOGY

Halborn performed a combination of manual and automated security testing to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of this audit. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of the bridge code and can quickly identify items that do not follow security best practices. The following phases and associated tools were used throughout the term of the audit:

    • Research into architecture and purpose

    • Smart contract manual code review and walkthrough

    • Graphing out functionality and contract logic/connectivity/functions. (solgraph)

    • Manual assessment of use and safety for the critical Solidity variables and functions in scope to identify any arithmetic related vulnerability classes

    • Manual testing by custom scripts

    • Scanning of solidity files for vulnerabilities, security hotspots or bugs. (MythX)

    • Static Analysis of security for scoped contract, and imported functions. (Slither)

    • Testnet deployment (Brownie, Remix IDE, Visual Studio Code)

4. SCOPE

IN-SCOPE: The security assessment was scoped to the Collection Offer smart contract, NFTfi audit-collection-offer-28-07-2022 branch:

    • DirectLoanFixedCollectionOffer.sol

Commit ID: 6c6fce28f47128d0410fb195142388bdf9e72763

The NFTfi's loan solution was already tested twice:

    • in November 2021 (NFTfi develop-v2.1-audit branch contracts)

    • in March 2022 (NFTfi v2.2-07-03-2022-audit)

OUT-OF-SCOPE: Other smart contracts in the repository, external libraries and economical attacks.

5. RISK METHODOLOGY

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

6. SCOPE

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

7. Assessment Summary & Findings Overview

Critical

0

High

0

Medium

0

Low

0

Informational

2

Impact x Likelihood

HAL-01

HAL-02

Security analysisRisk levelRemediation Date
GAS OVER-CONSUMPTION IN LOOPSInformationalAcknowledged
SOLC 0.8.4 COMPILER VERSION CONTAINS MULTIPLE BUGSInformationalAcknowledged

8. Findings & Tech Details

8.1 GAS OVER-CONSUMPTION IN LOOPS

// Informational

Description

In all the loops, the counter variable is incremented using i++. It is known that, in loops, using ++i costs less gas per iteration than i++.

Code Location

DirectLoanBaseMinimal.sol

  • Line 305: for (uint256 i = 0; i < _permittedErc20s.length; i++) {
  • Line 374: for (uint256 i = 0; i < _erc20s.length; i++) {

For example, based in the following test contract:

Test.sol

//SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

contract test {
    function postiincrement(uint256 iterations) public {
        for (uint256 i = 0; i < iterations; i++) {
        }
    }
    function preiincrement(uint256 iterations) public {
        for (uint256 i = 0; i < iterations; ++i) {
        }
    }
}

We can see the difference in the gas costs: posti_prei.png

Score
Impact: 1
Likelihood: 1
Recommendation

ACKNOWLEDGED: The \client team acknowledged this issue.

8.2 SOLC 0.8.4 COMPILER VERSION CONTAINS MULTIPLE BUGS

// Informational

Description

Presently, the smart contracts have configured the floating pragma set to ^0.8.0 or fixed pragma to 0.8.4 (e.g. DirectLoanFixedCollectionOffer.sol). The latest solidity compiler version 0.8.16 fixed important bugs in the compiler. The version 0.8.4 is missing all these fixes: 0.8.9, 0.8.13, 0.8.14, 0.8.15, 0.8.16. The official Solidity's recommendations are: when deploying contracts, you should use the latest released version of Solidity. Apart from exceptional cases, only the latest version receives security fixes.

Score
Impact: 1
Likelihood: 1
Recommendation

ACKNOWLEDGED: The \client team acknowledged this issue.

9. Review Notes

Halborn performed several manual tests in the DirectLoanFixedCollectionOffer.sol contract:

The manual tests were focused on testing the main functions of this contract:

  • acceptOffer()
  • getPayoffAmount()
  • updateMaximumLoanDuration()
  • updateAdminFee()
  • drainERC20Airdrop()
  • setERC20Permit()
  • setERC20Permits()
  • drainERC721Airdrop()
  • drainERC1155Airdrop()
  • mintObligationReceipt()
  • renegotiateLoan()
  • payBackLoan()
  • liquidateOverdueLoan()
  • pullAirdrop()
  • wrapCollateral()
  • cancelLoanCommitmentBeforeLoanHasBegun()
  • getWhetherNonceHasBeenUsedForUser()
  • getERC20Permit()

No significant issues were found during the manual tests.

10. Automated Testing

STATIC ANALYSIS REPORT

Description

Halborn used automated testing techniques to enhance the coverage of certain areas of the scoped contracts. Among the tools used was Slither, a Solidity static analysis framework. After Halborn verified all the contracts in the repository and was able to compile them correctly into their ABI and binary formats, Slither was run on the all-scoped contracts. This tool can statically verify mathematical relationships between Solidity variables to detect invalid or inconsistent usage of the contracts' APIs across the entire code-base.

Slither results

DirectLoanFixedCollectionOffer.sol

slither1.pngslither2.pngslither3.pngslither3_1.pngslither4.pngslither4_1.pngslither5.pngslither5_1.pngslither6.pngslither7.pngslither8.pngslither9.pngslither10.pngslither11.pngslither11_1.pngslither12.pngslither12_1.pngslither13.pngslither13_1.pngslither14.png
  • The reentrancy issues identified are false positives.

  • Some identified issues are related to OppenZepplin's libraries.

  • The dangerous comparison instances are false positives.

  • The uses inline assembly findings are false positives.

  • Pragma usage with old version was reported in SOLC 0.8.4 COMPILER VERSION CONTAINS MULTIPLE BUGS.

  • Several informational issues related to solidity naming convention were identified.

  • No major issues were found by Slither.

AUTOMATED SECURITY SCAN

Description

Halborn used automated security scanners to assist with detection of well-known security issues, and to identify low-hanging fruits on the targets for this engagement. Among the tools used was MythX, a security analysis service for Ethereum smart contracts. MythX performed a scan on all the contracts and sent the compiled results to the analyzers to locate any vulnerabilities.

MythX results

DirectLoanFixedCollectionOffer.sol

mythx1.pngmythx2.pngmythx3.pngmythx4.pngmythx5.png
  • Majority of identified issues are related to OppenZepplin's libraries.

  • The identified arithmetic operations or assert violations are false positives.

  • No major issues were found by Mythx.

Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.