Prepared by:
HALBORN
Last Updated 04/26/2024
Date of Engagement by: April 11th, 2022 - April 13th, 2022
100% of all REPORTED Findings have been addressed
All findings
4
Critical
0
High
0
Medium
0
Low
1
Informational
3
\client engaged Halborn to conduct a security audit on their smart contracts beginning on 2022-04-11 and ending on April 13th, 2022. The security assessment was scoped to the smart contracts provided to the Halborn team.
It should be noted that the assessment was timeboxed and only three days were provided to assess the security of the smart contracts before public release.
The smart contracts within the scope provided the functionality of swapping Seascape's NFTs via their web platform. Users are able to initiate a swap request by offering up to five NFTs plus an optional bounty, and request up to five NFTs. Users of the platform with the requested NFTs can accept the swap by transferring their NFTs in exchange for the offered ones plus the optional bounty.
The team at Halborn was provided three days 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 mostly addressed by the \client team.
Halborn performed a combination of manual and automated security testing to balance efficiency, timeliness, practicality, and accuracy regarding the scope of the smart contract audit. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of smart contracts and can quickly identify items that do not follow security best practices. The following phases and associated tools were used throughout the term of the audit:
Research into architecture and purpose.
Smart Contract manual code review and walkthrough.
Dynamic Analysis (ganache-cli, brownie, hardhat).
The assessment was scoped to the public GitHub repository on the \href{https://github.com/blocklords/seascape-smartcontracts/tree/nft-marketplace}{nft-marketplace branch}. The smart contracts in scope were inside the contracts/nft_swap directory.
The audited commit was 14a4eb1969eefa582a63dc3805186be08a38972f and fixes were assessed on newer commits.
Critical
0
High
0
Medium
0
Low
1
Informational
3
Impact x Likelihood
HAL-01
HAL-02
HAL-03
HAL-04
| Security analysis | Risk level | Remediation Date |
|---|---|---|
| MISSING REENTRANCY GUARD | Low | Solved - 04/12/2022 |
| NFT NUMBER 0 CAN NEVER BE TRADED | Informational | Solved - 04/12/2022 |
| UPGRADE TO AT LEAST PRAGMA 0.8.10 | Informational | Acknowledged |
| USING ++I CONSUMES LESS GAS THAN I++ IN LOOPS | Informational | Solved - 04/12/2022 |
// Low
During the audit, it was discovered that the cancelOffer function did not implement protections against reentrancy attacks. To protect against reentrancy attacks, it may be necessary to use a mutex. By using this lock, an attacker can no longer exploit the withdrawal function with a recursive call. OpenZeppelin has its own mutex implementation called ReentrancyGuard which provides a modifier to any function called nonReentrant that protects the function against re-entrancy attacks.
Halborn attempted to exploit a possible reentrancy vulnerability in the cancelOffer function. However, it was not possible to successfully exploit the issue due to the NFT transfer not being successful after deletion of the offer index on the second iteration of the re-entered function.
The affected function was cancelOffer on line 396.
SOLVED: The Seascape team added the nonReentrant modifier.
\newpage
// Informational
While currently supported NFTs start with tokenId equal to one, should additional NFT series be supported, with the first tokenId being zero, they would not be able to be traded.
Location: NftSwap.sol - function createOffer
require(_offeredTokens[index].tokenId > 0, "nft id must be greater than 0");
Location: NftSwap.sol - function acceptOffer
require(_requestedTokenIds[i] > 0, "nft id must be greater than 0");
SOLVED: The Seascape team removed the require statements.
\newpage
// Informational
Gas optimizations and additional safety checks are available for free when using newer compiler versions and the optimizer.
The contracts within scope made use of the pragma version 0.6.7
ACKNOWLEDGED: The Seascape team decided not to upgrade to the latest pragma versions and instead deploy with the current one. They confirmed that future projects will use the newest versions.
\newpage
// Informational
In the loop below, the variable i is incremented using i++. It is known that, in loops, using ++i costs less gas per iteration than i++. This does not only apply to the iterator variable. It also applies to variables declared within the loop code block.
NFTSwap.sol
SOLVED: The Seascape team now uses ++i to increment variables inside loops, saving some gas.
\newpage
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.
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed