Prepared by:
HALBORN
Last Updated 04/26/2024
Date of Engagement by: January 3rd, 2022 - January 10th, 2022
Summary
100% of all REPORTED Findings have been addressed
All findings
8
Critical
0
High
0
Medium
0
Low
2
Informational
6
Table of Contents
- 1. Introduction
- 2. Audit summary
- 3. Test approach & methodology
- 4. Scope
- 5. Risk methodology
- 6. Scope
- 7. Assessment summary & findings overview
- 8. Findings & Tech Details
- 8.1 Unchecked transfer
- 8.2 Missing zero address check
- 8.3 Possible misuse of public functions
- 8.4 Solc 0.8.4 compiler version contains multiple bugs
- 8.5 Constant keccak variables are treated as expressions, not constants
- 8.6 Using ++i consumes less gas than i++ in loops
- 8.7 Unneeded initialization of uint256 variables to 0
- 8.8 Uint32/uint64 types are less gas efficient
- 9. Review Notes
- 10. Automated Testing
1. INTRODUCTION
0xNodes engaged Halborn to conduct a security audit on their smart contracts beginning on January 3rd, 2021 and ending on January 10th. The security assessment was scoped to the smart contract provided in the Github repository 0xNODES/platform/tree/audit.
2. AUDIT SUMMARY
The team at Halborn was provided a week for the engagement and assigned a full time security engineer to audit the security of the smart contract. The security engineer is a blockchain and smart-contract security expert with advanced penetration testing, smart-contract hacking, and deep knowledge of multiple blockchain protocols.
The purpose of this audit is to:
Ensure that smart contract functions operate as intended
Identify potential security issues with the smart contracts
In summary, Halborn identified some security risks that were acknowledged by 0xNodes 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)
4. SCOPE
IN-SCOPE: The security assessment was scoped to the following smart contracts:
UniswapV3Integration.solKernel.solIntegrationMap.solInterconnects.solYieldManager.sol
Commit ID: 06a3dd2ddcec58f6a32bb479b4f1b79530eec257
5. RISK METHODOLOGY
- 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.
- 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.
- 10 - CRITICAL
- 9 - 8 - HIGH
- 7 - 6 - MEDIUM
- 5 - 4 - LOW
- 3 - 1 - VERY LOW AND INFORMATIONAL
6. SCOPE
7. Assessment Summary & Findings Overview
Critical
0
High
0
Medium
0
Low
2
Informational
6
Impact x Likelihood
HAL-01
HAL-02
HAL-03
HAL-04
HAL-05
HAL-06
HAL-07
HAL-08
| Security analysis | Risk level | Remediation Date |
|---|---|---|
| UNCHECKED TRANSFER | Low | Risk Accepted |
| MISSING ZERO ADDRESS CHECK | Low | Risk Accepted |
| POSSIBLE MISUSE OF PUBLIC FUNCTIONS | Informational | Acknowledged |
| SOLC 0.8.4 COMPILER VERSION CONTAINS MULTIPLE BUGS | Informational | Acknowledged |
| CONSTANT KECCAK VARIABLES ARE TREATED AS EXPRESSIONS, NOT CONSTANTS | Informational | Acknowledged |
| USING ++I CONSUMES LESS GAS THAN I++ IN LOOPS | Informational | Acknowledged |
| UNNEEDED INITIALIZATION OF UINT256 VARIABLES TO 0 | Informational | Acknowledged |
| UINT32/UINT64 TYPES ARE LESS GAS EFFICIENT | Informational | Acknowledged |
8. Findings & Tech Details
8.1 UNCHECKED TRANSFER
// Low
Description
In the contract UniswapV3Integration the return value of an external transfer call is not checked. Several tokens do not revert in case of failure and return false. If one of these tokens is used, in case of failure, the withdraw() call would not revert and the Kernel contract would not receive any funds.
Code Location
UniswapV3Integration.sol
UniswapV3Integration.sol
// Transfer the funds
IERC20MetadataUpgradeable(token).transfer(
moduleMap.getModuleAddress(Modules.Kernel),
transferAmount // Change to account for partial inavailability
);
Score
Likelihood: 1
Recommendation
RISK ACCEPTED:The 0xNodes team accepted the risk in this issue.
8.2 MISSING ZERO ADDRESS CHECK
// Low
Description
Some initialize functions are missing address validation. Every address should be validated and checked that is different from zero.
UniswapV3Integration.sol
- Line 51:
function initialize(address[] memory controllers_, address moduleMap_, address nonfungiblePositionManager_, address uniswapFactory_)
Kernel.sol
- Line 72:
function initialize(address admin_, address owner_, address moduleMap_) - Line 127:
function addIntegration(address contractAddress, string memory name) - Line 142:
function addToken(address tokenAddress, bool acceptingDeposits, bool acceptingWithdrawals, bool acceptingLping, bool acceptingBridging, uint256 biosRewardWeight, uint256 reserveRatioNumerator, uint256 targetLiquidityRatioNumerator, uint256 transferFeeKValueNumerator, uint256 transferFeePlatformRatioNumerator) - Line 314:
function updateTokenRewardWeight(address tokenAddress, uint256 updatedWeight) - Line 330:
function updateTokenReserveRatioNumerator(address tokenAddress, uint256 reserveRatioNumerator) - Line 349:
function updateTokenTargetLiquidityRatioNumerator(address tokenAddress, uint256 targetLiquidityRatioNumerator) - Line 367:
function updateTokenTransferFeeKValueNumerator(address tokenAddress, uint256 transferFeeKValueNumerator) - Line 386:
function updateTokenTransferFeePlatformRatioNumerator(address tokenAddress, uint256 transferFeePlatformRatioNumerator) - Line 404:
function updateGasAccount(address payable gasAccount) - Line 415:
function updateTreasuryAccount(address payable treasuryAccount) - Line 426:
function updateGasAccountTargetEthBalance(uint256 gasAccountTargetEthBalance)
IntegrationMap.sol
- Line 30:
function initialize(address[] memory controllers_, address moduleMap_, address wethTokenAddress_, address biosTokenAddress_) - Line 68:
function addIntegration(address contractAddress, string memory name) - Line 145:
function addToken(address tokenAddress, bool acceptingDeposits, bool acceptingWithdrawals, bool acceptingLping, bool acceptingBridging, uint256 biosRewardWeight, uint256 reserveRatioNumerator, uint256 targetLiquidityRatioNumerator, uint256 transferFeeKValueNumerator, uint256 transferFeePlatformRatioNumerator)
Interconnects.sol
- Line 37:
function initialize(address[] memory controllers_, address moduleMap_, address payable relayAccount_) - Line 75:
function updateRelayAccount(address payable relayAccount_)
YieldManager.sol
- Line 61:
function initialize(address[] memory controllers_, address moduleMap_, uint256 gasAccountTargetEthBalance_, uint32 biosBuyBackEthWeight_, uint32 treasuryEthWeight_, uint32 protocolFeeEthWeight_, uint32 rewardsEthWeight_, address payable gasAccount_, address payable treasuryAccount_) - Line 106:
function updateGasAccount(address payable gasAccount_) - Line 115:
function updateTreasuryAccount(address payable treasuryAccount_)
Score
Likelihood: 3
Recommendation
RISK ACCEPTED:The 0xNodes team accepted the risk in this issue.
8.3 POSSIBLE MISUSE OF PUBLIC FUNCTIONS
// Informational
Description
In the following contracts there are functions marked as public but they are never directly called within the same contract or in any of their descendants:
UniswapV3Integration.sol
initialize()(UniswapV3Integration.sol#51-62)
Kernel.sol
isManager()(Kernel.sol#644-646)isOwner(address)(Kernel.sol#650-652)isLiquidityProvider()(Kernel.sol#656-663)
IntegrationMap.sol
initialize()(IntegrationMap.sol#30-64)
Interconnects.sol
initialize()(Interconnects.sol#37-44)getRelayAccount()(Interconnects.sol#84-86)getTokenUserLpBalance()(Interconnects.sol#452-459)getTokenPoolLpBalance()(Interconnects.sol#462-469)getTokenUserLpFeeRewardBalance()(Interconnects.sol#490-497)getTokenLpUsers()(Interconnects.sol#500-507)getTokenProtocolFeeRewards()(Interconnects.sol#510-517)
YieldManager.sol
initialize()(YieldManager.sol#61-80)harvestYield()(YieldManager.sol#246-325)getReserveTokenBalance()(YieldManager.sol#500-515)getDesiredReserveTokenBalance()(YieldManager.sol#519-540)getProcessedWethByToken()(YieldManager.sol#577-584)getProcessedWethByTokenSum()(YieldManager.sol#587-598)getTokenTotalIntegrationBalance()(YieldManager.sol#602-623)getGasAccount()(YieldManager.sol#626-628)getTreasuryAccount()(YieldManager.sol#631-633)getLastEthRewardsAmount()(YieldManager.sol#636-638)getGasAccountTargetEthBalance()(YieldManager.sol#641-648)getEthDistributionWeights()(YieldManager.sol#654-671)
Score
Likelihood: 1
Recommendation
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
8.4 SOLC 0.8.4 COMPILER VERSION CONTAINS MULTIPLE BUGS
// Informational
Description
Solidity compiler version 0.8.9 fixed important bugs in the compiler. The version 0.8.4 is missing this fix:
Score
Likelihood: 1
Recommendation
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
8.5 CONSTANT KECCAK VARIABLES ARE TREATED AS EXPRESSIONS, NOT CONSTANTS
// Informational
Description
In the contract Kernel, the roles are declared the following way:
Kernel.sol
bytes32 public constant OWNER_ROLE = keccak256("owner_role");
bytes32 public constant MANAGER_ROLE = keccak256("manager_role");
Kernel.sol
bytes32 public constant LIQUIDITY_PROVIDER_ROLE =
keccak256("liquidity_provider_role");
This results in the keccak256 operation being performed whenever the variable is used, increasing gas costs relative to just storing the output hash.
Score
Likelihood: 1
Recommendation
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
8.6 USING ++I CONSUMES LESS GAS THAN I++ IN LOOPS
// Informational
Description
In all the loops, the variable i is incremented using i++. It is known that, in loops, using ++i costs less gas per iteration than i++.
Code Location
UniswapV3Integration.sol
- Line 298:
for (uint32 i = 1; i <= poolIDCounter; i++)
IntegrationMap.sol
- Line 540:
for (uint256 tokenId; tokenId < tokenAddresses.length; tokenId++) {
Interconnects.sol
- Line 65:
for (uint256 i = 0; i < tokens.length; i++) { - Line 111:
for (uint256 tokenId; tokenId < tokens.length; tokenId++) { - Line 164:
for (uint256 tokenId; tokenId < tokens.length; tokenId++) { - Line 190:
lpUserIdx++ - Line 226:
for (uint256 tokenId; tokenId < tokens.length; tokenId++) { - Line 283:
for (uint256 i; i < tokens.length; i++) { - Line 311:
for (uint256 i; i < tokens.length; i++) { - Line 329:
for (uint256 i; i < tokens.length; i++) { - Line 356:
for (uint256 tokenId; tokenId < tokens.length; tokenId++) { - Line 388:
lpUserIdx++ - Line 425:
for (uint256 tokenId; tokenId < tokens.length; tokenId++) { - Line 481:
for (uint256 lpUserIdx; lpUserIdx < lpUsers.length; lpUserIdx++) {
YieldManager.sol
- Line 133:
for (uint256 i = 0; i < deployments.length; i++) { - Line 138:
for (uint256 j = 0; j < deployments[i].tokens.length; j++) { - Line 272:
tokenIterator++ - Line 440:
for (uint256 tokenId; tokenId < tokenCount; tokenId++) { - Line 567:
for (uint256 tokenId; tokenId < tokenCount; tokenId++) { - Line 593:
for (uint256 tokenId; tokenId < tokenAddresses.length; tokenId++) { - Line 617:
integrationId++ - Line 706:
integrationId++
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:
Score
Likelihood: 1
Recommendation
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
8.7 UNNEEDED INITIALIZATION OF UINT256 VARIABLES TO 0
// Informational
Description
uint256 variables are already initialized to 0 by default. uint256 i = 0, for example, reassigns the 0 to i which wastes gas.
Code Location
UniswapV3Integration.sol
- Line 107:
uint256 amount0Withdrawn = 0; - Line 108:
uint256 amount1Withdrawn = 0; - Line 247:
uint256 amountA = 0; - Line 248:
uint256 amountB = 0;
Interconnects.sol
- Line 65:
for (uint256 i = 0; i < tokens.length; i++) { - Line 479:
uint256 sum = 0;
YieldManager.sol
- Line 133:
for (uint256 i = 0; i < deployments.length; i++) { - Line 138:
for (uint256 j = 0; j < deployments[i].tokens.length; j++) {
Score
Likelihood: 1
Recommendation
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
8.8 UINT32/UINT64 TYPES ARE LESS GAS EFFICIENT
// Informational
Description
uint32, uint64... variables are less gas efficient than uint256. Due to how the EVM natively works on 256-bit numbers, using for example 32-bit number introduces additional costs as the EVM has to properly enforce the limits of this smaller type.
This happens in multiple functions across the different smart contracts:
UniswapV3Integration.sol
- Line 37:
uint32 public override poolIDCounter; - Line 40:
mapping(uint32 => PositionNFT) internal pools; - Line 43:
mapping(uint32 => mapping(address => uint256)) public override balances; - Line 64:
function _verifyPoolAndTokens(address token, uint32 poolID) internal view { - Line 72:
modifier verifyPoolAndTokens(address token, uint32 poolID) { - Line 81:
uint32 poolID - Line 99:
uint32 poolID - Line 234:
function deploy(uint32 poolID) external override { - Line 239:
uint32 poolID, - Line 298:
for (uint32 i = 1; i <= poolIDCounter; i++) { - Line 317:
function getPoolBalance(uint32 poolID) - Line 337:
function getPool(uint32 poolID) - Line 391:
function getPendingYield(uint32 poolID)
Kernel.sol
- Line 103:
function setBiosRewardsDuration(uint32 biosRewardsDuration) - Line 223:
uint32 biosBuyBackEthWeight, - Line 224:
uint32 treasuryEthWeight, - Line 225:
uint32 protocolFeeEthWeight, - Line 226:
uint32 rewardsEthWeight
IntegrationMap.sol
- Line 15:
uint32 private constant RESERVE_RATIO_DENOMINATOR = 1_000_000; - Line 26:
uint32 private constant TARGET_LIQUIDITY_RATIO_DENOMINATOR = 1_000_000; - Line 27:
uint32 private constant TRANSFER_FEE_K_VALUE_DENOMINATOR = 1_000_000; - Line 28:
uint32 private constant TRANSFER_FEE_PLATFORM_RATIO_DENOMINATOR = 1_000_000; - Line 669:
returns (uint32) - Line 694:
returns (uint32) - Line 719:
returns (uint32) - Line 744:
returns (uint32)
Interconnects.sol
- Line 416:
uint32 targetLiquidityRatioDenominator = integrationMap - Line 418:
uint32 kValueDenominator = integrationMap - Line 420:
uint32 protocolFeeDenominator = integrationMap
YieldManager.sol
- Line 32:
uint32 private biosBuyBackEthWeight; - Line 33:
uint32 private treasuryEthWeight; - Line 34:
uint32 private protocolFeeEthWeight; - Line 35:
uint32 private rewardsEthWeight; - Line 65:
uint32 biosBuyBackEthWeight_, - Line 66:
uint32 treasuryEthWeight_, - Line 67:
uint32 protocolFeeEthWeight_, - Line 68:
uint32 rewardsEthWeight_, - Line 94:
uint32 biosBuyBackEthWeight_, - Line 95:
uint32 treasuryEthWeight_, - Line 96:
uint32 protocolFeeEthWeight_, - Line 97:
uint32 rewardsEthWeight_ - Line 547:
returns (uint32 ethWeightSum) - Line 659:
uint32, - Line 660:
uint32, - Line 661:
uint32, - Line 662:
uint32,
In general, the usage of these smaller types only improves the gas costs in cases where the variables can be packed together, for example structs.
Score
Likelihood: 1
Recommendation
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
9. Review Notes
CONTRACT INITIALIZATION
No major issues found in the initialization, although, it is recommended to initialize ReentrancyGuardUpgradeable in the Interconnects contract by calling __ReentrancyGuard_init().
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
Slither only found some issues in the following smart contracts:
Kernel.sol
IntegrationMap.sol
Interconnects.sol
YieldManager.sol
No major issues were found by Slither.
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.
Table of Contents
- 1. Introduction
- 2. Audit summary
- 3. Test approach & methodology
- 4. Scope
- 5. Risk methodology
- 6. Scope
- 7. Assessment summary & findings overview
- 8. Findings & Tech Details
- 8.1 Unchecked transfer
- 8.2 Missing zero address check
- 8.3 Possible misuse of public functions
- 8.4 Solc 0.8.4 compiler version contains multiple bugs
- 8.5 Constant keccak variables are treated as expressions, not constants
- 8.6 Using ++i consumes less gas than i++ in loops
- 8.7 Unneeded initialization of uint256 variables to 0
- 8.8 Uint32/uint64 types are less gas efficient
- 9. Review Notes
- 10. Automated Testing
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed