Audit name:
[SCA] Salvor | Auction | Sep2022
Date:
Sep 27, 2022
Table of Content
Introduction
We express our gratitude to the Salvor team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.
Salvor is an auction platform for ERC721Upgradeable tokens.
| title | content |
|---|---|
| Platform | AVM |
| Language | Solidity |
| Tags | ERC721, Marketplace, Auction |
| Timeline | 06/09/2022 - 28/09/2022 |
| Methodology | https://hackenio.cc/sc_methodology→ |
Review Scope | |
|---|---|
| Repository | https://gitlab.com/salvor1/salvor-contracts→ |
| Commit | d397df3587c27acca8e4ef08f31e471eb5caaf15 |
Review Scope
- Commit
- d397df3587c27acca8e4ef08f31e471eb5caaf15
Audit Summary
10/10
94.97%
9/10
10/10
The system users should acknowledge all the risks summed up in the risks section of the report
Document Information
This report may contain confidential information about IT systems and the intellectual property of the Customer, as well as information about potential vulnerabilities and methods of their exploitation.
The report can be disclosed publicly after prior consent by another Party. Any subsequent publication of this report shall be without mandatory consent.
Document | |
|---|---|
| Name | Smart Contract Code Review and Security Analysis Report for Salvor |
| Audited By | Hacken |
| Changelog | 16/09/2022 - Initial Review |
| 28/09/2022 - Second Review |
Document
- Name
- Smart Contract Code Review and Security Analysis Report for Salvor
- Audited By
- Hacken
- Changelog
- 16/09/2022 - Initial Review
- 28/09/2022 - Second Review
System Overview
Salvor is an auction platform for ERC721Upgradeable tokens (NFTs), and it provides various auction patterns with the contracts below.
AuctionMarketplace - an upgradable contract that allows NFT owners to start an English auction with “buy now” price option. Current bids are open to everyone. If a user wants to bid, the amount should be higher than the previous highest one, and the rise should match the required min bid increase amount.
PaymentManager - an upgradable contract that manages the transfer commissions, royalties, and revenue shares for every marketplace contract. Each time the NFT owner changes, it can be expected 2 different fees:
Commission fee
A fee type that is taken from every NFT sale, if a commission percentage is set by the owner.
Royalty fee
The second fee type is only taken if the NFT contract supports single or multi royalty standard(EIP-2981). The royalty fee will be distributed to the original NFT creator/creators.
IPaymentManager - an interface of the PaymentManager contract.
DutchAuctionMarketplace - an upgradable contract that provides a Dutch auction platform. Asset owner can start an auction at a starting price, which slowly decreases over time until the reserve price is reached. Auction immediately ends when a user catches the current Dutch price.
LibShareHolder - a library that helps to store shareholder addresses and their percentages.
LibOrder - a library that helps to store the order info and hash it.
Marketplace - an upgradable contract that allows users to list and buy NFTs and make/accept offers to NFTs. Users can bid for any NFT asset without requiring a whitelisting process.
INFTCollectible - an interface of the NFTCollectible contract.
NFTCollectible - an ERC721 token contract that supports Royalty feature.
IRoyalty - an interface of the Royalty contract.
LibRoyalty - a library used by Royalty contract to store royalty info.
Royalty - a helper contract that sets royalty addresses and percentages to signal a royalty amount to be paid to the NFT creator or rights holder every time the NFT is sold or re-sold.
Privileged roles
NFT owners can:
start an auction by choosing a type
settle the auction
cancel the auction if it has not any offer
Owner of the AuctionMarketplace contract can:
set payment manager address
set default bid increase percentage
set minimum price limit
set maximum duration period
set default auction bid period
pause/unpause the contract
Owner of the DutchAuctionMarketplace can:
set the payment manager address
pause/unpause the contract
set minimum price limit
set maximum duration
set minimum drop interval
Owner of the Marketplace contract can:
set the payment manager address
set minimum price limit
pause/unpause the contract
Owner of the NFTCollectible contract can:
set the base token URI
set the base extension
set the default royalties
update the royalties
mint tokens infinitely
Owner of the PaymentManager contract can:
add a marketplace address to the whitelist
remove a marketplace from the whitelist
set company wallet address
set commission percentage
pause/unpause the contract
Executive Summary
Documentation quality
The total Documentation quality score is 10 out of 10.
Functional requirements and a technical description for each contract were provided.
Code explanations are well written, and the documentation is public.
Code quality
The total Code quality score is 9 out of 10.
Style guide violation was found.
Marketplace platforms were split into contracts, and the general architecture was well-designed.
Test coverage
Code coverage of the project is 94.97%.
Basic user interactions and setting environments were covered.
Negative test cases were missing.
Security score
Upon auditing, the code was found to contain 1 critical, 3 high, 1 medium, and 9 low severity issues. Out of these, 13 issues have been addressed and resolved, leading to a Security score of 10 out of 10.
All identified issues are detailed in the “Findings” section of this report.
Summary
The comprehensive audit of the customer's smart contract yields an overall score of 9.45. This score reflects the combined evaluation of documentation, code quality, test coverage, and security aspects of the project.
Findings
Code ― | Title | Status | Severity | |
|---|---|---|---|---|
| F-2023-0548 | Compilation issues | fixed | Critical | |
| F-2023-0551 | Funds lock | fixed | High | |
| F-2023-0550 | Highly permissive owner access | fixed | High | |
| F-2023-0549 | Highly permissive owner access | fixed | High | |
| F-2023-0552 | Requirement compliance violation / Wrong | fixed | Medium | |
| F-2023-0561 | Style guide violation | unfixed | Low | |
| F-2023-0560 | Boolean equality | fixed | Low | |
| F-2023-0559 | Missing zero address validation | fixed | Low | |
| F-2023-0558 | Functions that can be declared as external | fixed | Low | |
| F-2023-0557 | State variables' default visibility | fixed | Low |
Appendix 1. Severity Definitions
When auditing smart contracts, Hacken is using a risk-based approach that considers Likelihood, Impact, Exploitability and Complexity metrics to evaluate findings and score severities.
Reference on how risk scoring is done is available through the repository in our Github organization:
Severity | Description |
|---|---|
Critical | Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation. |
High | High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation. |
Medium | Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category. |
Low | Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution, do not affect security score but can affect code quality score. |
Severity
- Critical
Description
- Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation.
Severity
- High
Description
- High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation.
Severity
- Medium
Description
- Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category.
Severity
- Low
Description
- Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution, do not affect security score but can affect code quality score.
Appendix 2. Scope
The scope of the project includes the following smart contracts from the provided repository:
Scope Details | |
|---|---|
| Repository | https://gitlab.com/salvor1/salvor-contracts→ |
| Commit | d397df3587c27acca8e4ef08f31e471eb5caaf15 |
| Whitepaper | Provided |
| Requirements | Provided→ |
| Technical Requirements | Provided→ |