Prepared by:
HALBORN
Last Updated 04/26/2024
Date of Engagement by: January 3rd, 2022 - January 10th, 2022
100% of all REPORTED Findings have been addressed
All findings
8
Critical
0
High
0
Medium
0
Low
2
Informational
6
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.
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.
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)
IN-SCOPE: The security assessment was scoped to the following smart contracts:
UniswapV3Integration.sol
Kernel.sol
IntegrationMap.sol
Interconnects.sol
YieldManager.sol
Commit ID: 06a3dd2ddcec58f6a32bb479b4f1b79530eec257
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 |
// Low
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.
UniswapV3Integration.sol
// Transfer the funds
IERC20MetadataUpgradeable(token).transfer(
moduleMap.getModuleAddress(Modules.Kernel),
transferAmount // Change to account for partial inavailability
);
RISK ACCEPTED:The 0xNodes team accepted the risk in this issue.
// Low
Some initialize functions are missing address validation. Every address should be validated and checked that is different from zero.
UniswapV3Integration.sol
function initialize(address[] memory controllers_, address moduleMap_, address nonfungiblePositionManager_, address uniswapFactory_)Kernel.sol
function initialize(address admin_, address owner_, address moduleMap_)function addIntegration(address contractAddress, string memory name)function addToken(address tokenAddress, bool acceptingDeposits, bool acceptingWithdrawals, bool acceptingLping, bool acceptingBridging, uint256 biosRewardWeight, uint256 reserveRatioNumerator, uint256 targetLiquidityRatioNumerator, uint256 transferFeeKValueNumerator, uint256 transferFeePlatformRatioNumerator)function updateTokenRewardWeight(address tokenAddress, uint256 updatedWeight)function updateTokenReserveRatioNumerator(address tokenAddress, uint256 reserveRatioNumerator)function updateTokenTargetLiquidityRatioNumerator(address tokenAddress, uint256 targetLiquidityRatioNumerator)function updateTokenTransferFeeKValueNumerator(address tokenAddress, uint256 transferFeeKValueNumerator)function updateTokenTransferFeePlatformRatioNumerator(address tokenAddress, uint256 transferFeePlatformRatioNumerator)function updateGasAccount(address payable gasAccount)function updateTreasuryAccount(address payable treasuryAccount)function updateGasAccountTargetEthBalance(uint256 gasAccountTargetEthBalance)IntegrationMap.sol
function initialize(address[] memory controllers_, address moduleMap_, address wethTokenAddress_, address biosTokenAddress_)function addIntegration(address contractAddress, string memory name)function addToken(address tokenAddress, bool acceptingDeposits, bool acceptingWithdrawals, bool acceptingLping, bool acceptingBridging, uint256 biosRewardWeight, uint256 reserveRatioNumerator, uint256 targetLiquidityRatioNumerator, uint256 transferFeeKValueNumerator, uint256 transferFeePlatformRatioNumerator)Interconnects.sol
function initialize(address[] memory controllers_, address moduleMap_, address payable relayAccount_)function updateRelayAccount(address payable relayAccount_)YieldManager.sol
function initialize(address[] memory controllers_, address moduleMap_, uint256 gasAccountTargetEthBalance_, uint32 biosBuyBackEthWeight_, uint32 treasuryEthWeight_, uint32 protocolFeeEthWeight_, uint32 rewardsEthWeight_, address payable gasAccount_, address payable treasuryAccount_)function updateGasAccount(address payable gasAccount_)function updateTreasuryAccount(address payable treasuryAccount_)RISK ACCEPTED:The 0xNodes team accepted the risk in this issue.
// Informational
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)ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
// Informational
Solidity compiler version 0.8.9 fixed important bugs in the compiler. The version 0.8.4 is missing this fix:
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
// Informational
In the contract Kernel, the roles are declared the following way:
bytes32 public constant OWNER_ROLE = keccak256("owner_role");
bytes32 public constant MANAGER_ROLE = keccak256("manager_role");
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.
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
// Informational
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++.
UniswapV3Integration.sol
for (uint32 i = 1; i <= poolIDCounter; i++)IntegrationMap.sol
for (uint256 tokenId; tokenId < tokenAddresses.length; tokenId++) {Interconnects.sol
for (uint256 i = 0; i < tokens.length; i++) {for (uint256 tokenId; tokenId < tokens.length; tokenId++) {for (uint256 tokenId; tokenId < tokens.length; tokenId++) {lpUserIdx++for (uint256 tokenId; tokenId < tokens.length; tokenId++) {for (uint256 i; i < tokens.length; i++) {for (uint256 i; i < tokens.length; i++) {for (uint256 i; i < tokens.length; i++) {for (uint256 tokenId; tokenId < tokens.length; tokenId++) {lpUserIdx++for (uint256 tokenId; tokenId < tokens.length; tokenId++) {for (uint256 lpUserIdx; lpUserIdx < lpUsers.length; lpUserIdx++) {YieldManager.sol
for (uint256 i = 0; i < deployments.length; i++) {for (uint256 j = 0; j < deployments[i].tokens.length; j++) {tokenIterator++for (uint256 tokenId; tokenId < tokenCount; tokenId++) {for (uint256 tokenId; tokenId < tokenCount; tokenId++) {for (uint256 tokenId; tokenId < tokenAddresses.length; tokenId++) {integrationId++integrationId++For example, based in the following test contract:
//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:
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
// Informational
uint256 variables are already initialized to 0 by default. uint256 i = 0, for example, reassigns the 0 to i which wastes gas.
UniswapV3Integration.sol
uint256 amount0Withdrawn = 0;uint256 amount1Withdrawn = 0;uint256 amountA = 0;uint256 amountB = 0;Interconnects.sol
for (uint256 i = 0; i < tokens.length; i++) {uint256 sum = 0;YieldManager.sol
for (uint256 i = 0; i < deployments.length; i++) {for (uint256 j = 0; j < deployments[i].tokens.length; j++) {ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
// Informational
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
uint32 public override poolIDCounter;mapping(uint32 => PositionNFT) internal pools;mapping(uint32 => mapping(address => uint256)) public override balances;function _verifyPoolAndTokens(address token, uint32 poolID) internal view {modifier verifyPoolAndTokens(address token, uint32 poolID) {uint32 poolIDuint32 poolIDfunction deploy(uint32 poolID) external override {uint32 poolID,for (uint32 i = 1; i <= poolIDCounter; i++) {function getPoolBalance(uint32 poolID)function getPool(uint32 poolID)function getPendingYield(uint32 poolID)Kernel.sol
function setBiosRewardsDuration(uint32 biosRewardsDuration)uint32 biosBuyBackEthWeight,uint32 treasuryEthWeight,uint32 protocolFeeEthWeight,uint32 rewardsEthWeightIntegrationMap.sol
uint32 private constant RESERVE_RATIO_DENOMINATOR = 1_000_000;uint32 private constant TARGET_LIQUIDITY_RATIO_DENOMINATOR = 1_000_000;uint32 private constant TRANSFER_FEE_K_VALUE_DENOMINATOR = 1_000_000;uint32 private constant TRANSFER_FEE_PLATFORM_RATIO_DENOMINATOR = 1_000_000;returns (uint32)returns (uint32)returns (uint32)returns (uint32)Interconnects.sol
uint32 targetLiquidityRatioDenominator = integrationMapuint32 kValueDenominator = integrationMapuint32 protocolFeeDenominator = integrationMapYieldManager.sol
uint32 private biosBuyBackEthWeight;uint32 private treasuryEthWeight;uint32 private protocolFeeEthWeight;uint32 private rewardsEthWeight;uint32 biosBuyBackEthWeight_,uint32 treasuryEthWeight_,uint32 protocolFeeEthWeight_,uint32 rewardsEthWeight_,uint32 biosBuyBackEthWeight_,uint32 treasuryEthWeight_,uint32 protocolFeeEthWeight_,uint32 rewardsEthWeight_returns (uint32 ethWeightSum)uint32,uint32,uint32,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.
ACKNOWLEDGED:The 0xNodes team acknowledged this issue.
No major issues found in the initialization, although, it is recommended to initialize ReentrancyGuardUpgradeable in the Interconnects contract by calling __ReentrancyGuard_init().
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 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.
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed