Introduction
We express our gratitude to the Kyber Network team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.
Velocore is a groundbreaking solution, synthesizing top-tier DEX methodologies into a novel, high-performance flywheel
title | content |
---|---|
Platform | Ethereum, BNB Chain, Arbitrum, Polygon, Fantom, Avalanche, Optimism |
Language | Solidity |
Tags | Governance |
Timeline | 19/03/2021 – 04/04/2021 |
Methodology | https://hackenio.cc/sc_methodology→ |
Review Scope | |
---|---|
Repository | https://github.com/KyberNetwork/dao_sc/tree/katana→ |
Commit | 616906ABEED505F8D29186E147EB6736A914F49A |
Review Scope
- Commit
- 616906ABEED505F8D29186E147EB6736A914F49A
Audit Summary
According to the assessment, the Customer's smart contracts are secure.
Our team performed an analysis of code functionality, manual audit, and automated checks with Mythril and Slither. All issues found during automated analysis were manually reviewed, and important vulnerabilities are presented in the Audit overview section. A general overview is presented in AS-IS section, and all found issues can be found in the Audit overview section.
Security engineers found 1 high, 5 medium, and 1 informational issue during the audit. All issues are acknowledged and accepted by the Customer.
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 Kyber Network |
Audited By | Hacken |
Approved By | Hacken |
Changelog | 23/03/2021 – Initial Audit |
04/04/2021 – Adding Customer Notice |
Document
- Name
- Smart Contract Code Review and Security Analysis Report for Kyber Network
- Audited By
- Hacken
- Approved By
- Hacken
- Changelog
- 23/03/2021 – Initial Audit
- 04/04/2021 – Adding Customer Notice
System Overview
ASIS Overview
KyberGovernancesol
Description
KyberGovernance is a governance contract.
Imports
KyberGovernance has following imports:
import {SafeMath} from ‘@openzeppelin/contracts/math/SafeMath.sol’
import {PermissionAdmin} from ‘@kyber.network/utilssc/contracts/PermissionAdmin.sol’
import {IKyberGovernance} from ‘../interfaces/governance/IKyberGovernance.sol’
import {IExecutorWithTimelock} from ‘../interfaces/governance/IExecutorWithTimelock.sol’
import {IVotingPowerStrategy} from ‘../interfaces/governance/IVotingPowerStrategy.sol’
import {IProposalValidator} from ‘../interfaces/governance/IProposalValidator.sol’
import {getChainId} from ‘../misc/Helpers.sol’
Inheritance
KyberGovernance is IKyberGovernance, PermissionAdmin.
Usages
KyberGovernance contract has following usages
SafeMath for uint256.
Structs
KyberGovernance contract has no custom data structures.
Enums
KyberGovernance contract has no custom enums.
Events
KyberGovernance contract has no custom events.
Modifiers
KyberGovernance has no custom modifiers.
Fields
KyberGovernance contract has following fields and constants:
bytes32 public constant DOMAIN_TYPEHASH = keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)');
bytes32 public constant VOTE_EMITTED_TYPEHASH = keccak256('VoteEmitted(uint256 id,uint256 optionBitMask)');
string public constant NAME = 'Kyber Governance';
address private _daoOperator;
uint256 private _proposalsCount;
mapping(uint256 => Proposal) private _proposals;
mapping(address => bool) private _authorizedExecutors;
mapping(address => bool) private _authorizedVotingPowerStrategies;
Functions
KyberGovernance has following public and external functions:
constructor Description: \-Initializes the contract. Sets adimin and dao operator addresses. Executors and
votingStratages
are also provided. Visibility: \-public Input parameters: \-address admin
\-address daoOperator
\-address[] memory
executors \-address[] memory votingPowerStrategie
Constraints: \-None Events emit: \-Emits theExecutorAuthorized
andVotingPowerStrategyAuthorized
events. Output: \-NonecreateBinaryProposal Description: \-Creates a Binary Proposal. Visibility: \-external Input parameters: \-IExecutorWithTimelock executor \-IVotingPowerStrategy strategy \-
string[] memory options
\-uint256 startTime
\-uint256 endTime
\-string memory link
Constraints: \-executor should be authorized. \-strategy should be authorized. \-executor should validate input params. Events emit: \-Emits theBinaryProposalCreated
event. Output: \-uint256 proposalId
createBinaryProposal Description: \-Creates a Binary Proposal. Creates a Generic Proposal. It only gets the winning option without any executions. Visibility: \-external Input parameters: \-IExecutorWithTimelock executor \-IVotingPowerStrategy strategy \-
BinaryProposalParams memory executionParams
\-uint256 startTime
\-uint256 endTime
\-string memory link
Constraints: \-executionParams
should be provided. \-executionParams
should be of the same length. \-executor should be authorized. \-strategy should be authorized. \-executor should validate input params. Events emit: \-Emits theGenericProposalCreated
event. Output: \-uint256 proposalId
cancel Description: Cancels a Proposal. In the current version is only callable by the
_daoOperator
. Though, if if the IProposalValidator implementation will allow, the function may be callable by anyone. Visibility: \-external Input parameters: \-uint256 proposalId
Constraints: \-Proposal should exist. \-Should not be in a final state. Events emit: \-Emits theProposalCanceled
event. Output: \-Nonequeue Description: \-Queue a proposal if it is succeeded. Visibility: \-external Input parameters: \-
uint256 proposalId
Constraints: \-Proposal should exist. \-Proposal should be Binary. \-Should be in the Succeeded state. Events emit: \-Emits theProposalQueued
event. Output: \-Noneexecute Description: \-Execute a proposal. Visibility: \-external Input parameters: \-
uint256 proposalId
Constraints: \-Proposal should exist. \-Proposal should be Binary. \-Should be in the Queued state. Events emit: \-Emits theProposalExecuted
event. Output: \-NonesubmitVote Description: \-Submit a vote for a proposal by message sender. Visibility: \-external Input parameters: \-
uint256 proposalId
\-uint256 optionBitMask
Constraints: \-Proposal should exist. \-Should be in the Active state. Events emit: \-Emits theVoteEmittedevent
. Output: \-NonesubmitVoteBySignature Description: \-Submit a vote for a proposal by signature. Visibility: \-external Input parameters: \-
uint256 proposalId
\-uint256 optionBitMask
\-uint8 v
\-bytes32 r
\-bytes32 s
Constraints: \-Proposal should exist. \-Signature should be valid. \-Should be in the Active state. Events emit: \-Emits theVoteEmittedevent
. Output: \-NonehandleVotingPowerChanged Description: \-Changes voting power of a voter. Visibility: \-external Input parameters: \-
uint256 proposalId
\-uint256 optionBitMask
\-uint8 v
\-bytes32 r
\-bytes32 s
Constraints: \-Should be called from a strategy contract Events emit: \-Emits theVotingPowerChanged
. Output: \-NonetransferDaoOperator Description: \-Transfers dao operator to another address. Should be called from a current dao operator.
authorizeExecutors, unauthorizeExecutors, authorizeVotingPowerStrategies, unauthorizeVotingPowerStrategies Description: \-Admin functions to change corresponding contract parameters.
isExecutorAuthorized, isVotingPowerStrategyAuthorized, getDaoOperator, getProposalsCount, getProposalById, getProposalVoteDataById, getVoteOnProposal, getProposalStat Description: \-Simple view functions.
EpochVotingPowerStrategysol
Description
EpochVotingPowerStrategy – Voting Power Strategy contract based on epoch mechanism.
Imports
EpochVotingPowerStrategy has following imports:
import {SafeMath} from ‘@openzeppelin/contracts/math/SafeMath.sol’
import {IVotingPowerStrategy} from ‘../../interfaces/governance/IVotingPowerStrategy.sol’
import {IKyberGovernance} from ‘../../interfaces/governance/IKyberGovernance.sol’
import {IKyberStaking} from ‘../../interfaces/staking/IKyberStaking.sol’
import {EpochUtils} from '../../misc/EpochUtils.sol' Inheritance EpochVotingPowerStrategy is IVotingPowerStrategy, EpochUtils. Usages EpochVotingPowerStrategy contract has following usages:
SafeMath for uint256.
Structs
EpochVotingPowerStrategy contract has no custom data structures.
Enums
EpochVotingPowerStrategy contract has no custom enums.
Events
EpochVotingPowerStrategy contract has no custom events.
Modifiers
EpochVotingPowerStrategy has following custom modifiers: \-onlyStaking
– check if a caller is staking. \-onlyGovernance
– check if a caller is governance.
Fields
EpochVotingPowerStrategy contract has following fields and constants: \-uint256 public constant MAX_PROPOSAL_PER_EPOCH = 10
\-IKyberStaking public immutable staking
\-IKyberGovernance public immutable governance
\-mapping(uint256 => uint256[]) internal epochProposals
Functions
EpochVotingPowerStrategy has following public and external functions:
constructor Description: \-Initializes the contract. Sets governance and staking addresses. public Input parameters: \-
IKyberGovernance _governance
\-IKyberStaking _staking
Constraints: \-None Events emit: \-None Output: \-NonehandleProposalCreation Description: \-Add a proposal ID to a current epoch. Input parameters: \-
uint256 proposalId
\-uint256 startTime
\-uint256
– parameter exists but not used to be compliant with the interface. Constraints: \-Should only be called from a governance contract. Events emit: \-None Output: \-NonehandleProposalCancellation Description: \-Removes a proposal ID from a current epoch. Input parameters: \-
uint256 proposalId
Constraints: \-Should only be called from a governance contract. Events emit: \-None Output: \-NonehandleProposalCancellation Description: \-Returns voter's voting power. Input parameters: \-
address voter
\-uint256
\-uint256
Constraints: \-Should only be called from a governance contract. Events emit: \-None Output: \-uint256 votingPower
handleWithdrawal Description: \-Handle user withdraw from staking contract. Input parameters: \-
address user
\-uint256 /*reduceAmount*/
Constraints: \-Should only be called from a staking contract. Events emit: \-None Output:getVotingPower, validateProposalCreation, getMaxVotingPower, getListProposalIds Description: \-Simple view functions.
DefaultProposalValidatorsol
Description
DefaultProposalValidator is a contract with view or pure functions used to validate values or to retrieve data.
DefaultExecutorWithTimelocksol
Description
DefaultExecutorWithTimelock is a contract that can queue, execute, cancel transactions voted by Governance.
Imports
DefaultExecutorWithTimelock has following imports:
import {IExecutorWithTimelock} from ‘../../interfaces/governance/IExecutorWithTimelock.sol’
import {IKyberGovernance} from ‘../../interfaces/governance/IKyberGovernance.sol’
import {SafeMath} from ‘@openzeppelin/contracts/math/SafeMath.sol’
Inheritance
DefaultExecutorWithTimelock is IExecutorWithTimelock.
Usages
DefaultExecutorWithTimelock contract has following usages:
SafeMath for uint256.
Structs
DefaultExecutorWithTimelock contract has no custom data structures.
Enums
DefaultExecutorWithTimelock contract has no custom enums.
Events
DefaultExecutorWithTimelock contract has no custom events.
Modifiers
DefaultExecutorWithTimelock has following modifiers:
onlyAdmin
– check for a corresponding caller.onlyTimelock
– check for a corresponding caller.onlyPendingAdmin
– check for a corresponding caller.
Fields
DefaultExecutorWithTimelock contract has following fields and constants:
uint256 public immutable override GRACE_PERIOD
uint256 public immutable override MINIMUM_DELAY
uint256 public immutable override MAXIMUM_DELAY
address private _admin
address private _pendingAdmin
uint256 private _delay
mapping(bytes32 => bool) private _queuedTransactions
Functions
DefaultExecutorWithTimelock has following public and external functions:
constructor Description: \-Initializes the contract. Sets admin, delay, grace period for queued txs,
MINIMUM_DELAY
andMAXIMUM_DELAY
values. Visibility: \-public Input parameters: \-address admin \-uint256 delay \-uint256 gracePeriod \-uint256 minimumDelay \-uint256 maximumDelay Constraints: \-delay should be betweenminimumDelay
andmaximumDelay
. Events emit: \-Emits theNewDelay
andNewAdmin
events. Output: \-NonesetDelay, setPendingAdmin Description: \-Allows the contract to change corresponding parameters.
acceptAdmin Description: \-Allows pending admin to accept his permissions.
queueTransaction Description: \-Queues a transaction for execution. Visibility: \-public Input parameters: \-
address target
\-uint256 value
\-string memory signature
\-bytes memory data
\-uint256 executionTime
\-bool withDelegatecall
Constraints: \-Could only be called from the admin account. \-executionTime
should exceed or be equal to a current time + delay. Events emit: \-Emits theQueuedAction
event. Output: \-bytes32
– action hash.cancelTransaction Description: \-Cancel a transaction. Visibility: \-public Input parameters: \-
address target
\-uint256 value
\-string memory signature
\-bytes memory data
\-uint256 executionTime
\-bool withDelegatecall
Constraints: \-Could only be called from the admin account. Events emit: \-Emits theCancelledAction
event. Output: \-bytes32
– action hash.executeTransaction Description: \-Execute a transaction. Visibility: \-public Input parameters: \-
address target
\-uint256 value
\-string memory signature
\-bytes memory data
\-uint256 executionTime
\-bool withDelegatecall
Constraints: \-Could only be called from the admin account. \-Transaction should be queued. \-Current time should be after execution time and before grace period pass. \-Transaction should be sent successfully. Events emit Emits theExecutedAction
event. Output: \-bytes memory
getAdmin, getPendingAdmin, getDelay, isActionQueued, isProposalOverGracePeriod Description: \-Simple view functions.
Conclusion
Smart contracts within the scope were manually reviewed and analyzed with static analysis tools. For the contract, high-level description of functionality was presented in As-Is overview section of the report.
Audit report contains all found security vulnerabilities and other issues in the reviewed code. Security engineers found 1 high, 5 medium, and 1 informational issue during the audit.
All issues are acknowledged and accepted by the Customer.
Risks
The audit scope is limited to governance contracts. Its results may not be extrapolated to another contracts in the repository.
Writing universal contracts that can be used with different multiple implementations is considered a bad practice in solidity contracts. Such approach usually brings extra complexity and gas consumption.
Findings
Code ― | Title | Status | Severity | |
---|---|---|---|---|
F-2021-0109 | Admin can be set by voting | accepted | High | |
F-2021-011 | No gas limit validation | accepted | Medium | |
F-2021-0113 | Improper convergence | accepted | Medium | |
F-2021-011 | No upper or lower limits validation for constants on initialization | accepted | Medium | |
F-2021-0111 | No upper or lower limits validation for constants on initialization | accepted | Medium | |
F-2021-0110 | Reviewed contracts have high cohesion | accepted | Medium | |
I-2021-0104 | Style issues | accepted | Observation |
Identify vulnerabilities in your smart contracts.
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://github.com/KyberNetwork/dao_sc/tree/katana→ |
Commit | 616906ABEED505F8D29186E147EB6736A914F49A |
Scope Details
- Commit
- 616906ABEED505F8D29186E147EB6736A914F49A
Contracts in Scope
contracts/name1./contracts/governance/KyberGovernance.sol
contracts/governance/votingPowerStrategy/EpochVotingPowerStrategy.sol
contracts/governance/executor/DefaultProposalValidator.sol
contracts/governance/executor/DefaultExecutorWithTimelock.sol
contracts/governance/executor/DefaultExecutor.solsol