iosiro was commissioned by Synthetix to conduct a smart contract audit on portions of the Synthetix stablecoin system. The audit was performed between 14 August 2019 and 25 September 2019.
This report is organised into the following sections.
The information in this report should be used to understand the risk exposure of the smart contracts, and as a guide to improve the security posture of the smart contracts by remediating the issues that were identified. The results of this audit are only a reflection of the source code reviewed at the time of the audit and of the source code that was determined to be in-scope.
This report presents the findings of a second audit performed by iosiro on the Synthetix smart contracts. This iteration of the audit was focused on an updated version of the codebase and new scope. The report for the first iteration of the audit can be found here.
The purpose of this audit was to achieve the following.
Assessing the economics, game theory, or underlying business model of the platform were beyond the scope of this audit.
Due to the unregulated nature and ease of transfer of cryptocurrencies, operations that store or interact with these assets are considered very high risk with regards to cyber attacks. As such, the highest level of security should be observed when interacting with these assets. This requires a forward-thinking approach, which takes into account the new and experimental nature of blockchain technologies. There are a number of techniques that can help to achieve this, some of which are described below.
The second iteration of the audit had a modified scope and updated features. Notable changes since the first audit included:
At the conclusion of the audit one high risk issue, three low risk issues, and several informational level issues and design comments were open. The high risk issue pertained to the slashing defense mechanism used to protect against front-running, which could inadvertently result in the loss of legitimate user funds. The impact of the low risk issues included:
The informational level findings included excessive owner and oracle privileges, a function susceptible to integer overflow, as well as minor deviations from best practice and suggestions that may improve the functionality or readability of the code.
In line with the previous audit, the code was once again found to be of a very high standard, as it was modularised, well-documented, defensive, and generally adhered to best practices.
The new proxy pattern that was implemented to permit third party apps to call ERC20 functionality through it was found to operate as intended.
It should be noted that some core components were explicitly omitted from the scope of the assessment, including:
The risk posed by the smart contracts can be further mitigated by using the following controls prior to releasing the contracts in a production environment.
The source code considered in-scope for the assessment is described below. Code from all other files is considered to be out-of-scope. Out-of-scope code that interacts with in-scope code is assumed to function as intended and introduce no functional or security vulnerabilities for the purposes of this audit.
Project Name: Synthetix
Files: DelegateApprovals.sol, ExchangeGasPriceLimit.sol, EternalStorage.sol, ExchangeRates.sol, FeePool.sol, FeePoolEternalStorage.sol, FeePoolState.sol, Proxyable.sol, ProxyERC20.sol, PurgeableSynth.sol, RewardEscrow.sol, SupplySchedule.sol, Synthetix.sol
A variety of techniques were used while conducting the audit. These techniques are briefly described below.
The source code was manually inspected to identify potential security flaws. Code review is a useful approach for detecting security flaws, discrepancies between the specification and implementation, design improvements, and high risk areas of the system.
The contracts were compiled, deployed, and tested in a Ganache test environment, both manually and through the Truffle test suite provided. Manual analysis was used to confirm that the code operated at a functional level, and to verify the exploitability of any potential security issues identified. The coverage report generated by solidity-coverage of the Truffle tests included in the project is given below.
|File||% Statements||% Branch||% Functions||% Lines|
Tools were used to automatically detect the presence of several types of security vulnerabilities, including reentrancy, timestamp dependency bugs, and transaction-ordering dependency bugs. The static analysis results were manually analysed to remove false positive results. True positive results would be indicated in this report. Static analysis was conducted using Slither, Securify, as well as MythX. Tools such as the Remix IDE, compilation output, and linters were also used to identify potential areas of concern.
Each issue identified during the audit has been assigned a risk rating. The rating is determined based on the criteria outlined below.
The following section outlines the intended functionality of the system at a high level.
The Synthetix contract is described below.
The token should be ERC20 compliant with the following values.
|Name||Synthetix Network Token|
A user should be able to lock SNX in the Synthetix system in order to issue a proportionate number of synths, up to the issuance ratio. The user should then earn fees from transactions of synths within the system, relative to their percentage stake in the system as a whole.
If the price of SNX increases such that the collateralisation ratio for a user goes below the minimum specified amount, a portion of their locked tokens should be unlocked up to the point where the collateralisation ratio reaches the minimum required collateralisation ratio again.
Alternatively, if the price of SNX drops, then the user’s SNX should be locked within the contract and fee withdrawal disabled until either the price corrects itself or the user purchases enough SNX to match the required collateralisation ratio. In order to unlock the SNX, the user’s collateralisation ratio must be below the issuance ratio. To allow fee withdrawals, the user’s collateralisation ratio must be equal to or below
issuanceRatio * (1 + TARGET_THRESHOLD).
Fees collected through transfers and exchanges of synths should be tracked through a fee pool. The fee pool should track when SNX are locked within the system to provide collateral for the synths. Fees should be paid out proportionately after each period, which is within a defined fee period, to users depending on the amount of SNX locked and when they locked their tokens.
It should be possible for the Synthetix system to mint SNX token rewards based on a predefined issuance supply schedule. Any user should be able to call the minting function when a supply of reward tokens becomes mintable and receive a minter reward for doing so. The SNX rewards minted should then be placed in a reward escrow system for one year after the claim date. Users should be able to withdraw their SNX rewards after the escrow period has elapsed. The rewards supply schedule should be as indicated in the following table.
It should be possible for the Synthetix system to integrate with an escrow system.
Note: This functionality was beyond the scope of this assessment.
It should be possible for the owner of the Synthetix contract to add and remove synths to the Synthetix system.
It should be possible for the Synthetix system to issue or burn synths.
It should be possible for the Synthetix system to integrate with an exchange rate contract, which should control the various exchange rates within the system. The rates should be set by an oracle contract. Rates should be considered stale if they have not been updated in more than the time specified in the contract. A holder of synths should be able to exchange their synths held in one currency key into another currency denominated Synth flavour for an exchange fee. It should also be possible to set inverse pricing rates for inverse equivalent currencies known as inverse synths. These inverse synths should allow users to short the price of their representative (non-inverted) currency. Inverse synths should also have oracle-defined liquidation price margins above and below the starting value.
The oracle should maintain exchange rates within the system. It should also activate the protection circuit defense in the event that front-running transactions are detected.
Note: The oracle code was not available and the functionality was beyond the scope of this assessment.
*NOTE: This feature is now deprecated and no longer available
It should be possible for users to set a preferred currency, automatically converting any funds that they receive in synths to their preferred currency. If no preferred currency is set for a receiving address, the default currency should be the original currency. No fee should be charged on this exchange, as the user should be charged a fee for the transfer.
The Synthetix Drawing Rights is a flavour of synths that is loosely based on Special Drawing Rights. Its exchange rate is based on a basket aggregate of currencies to minimise exposure towards any particular fiat currency. Fees are stored in this currency, and users can hold these synths if they want to lessen their exposure to any particular fiat currency.
It should be possible to store the token balances in an external state contract to allow for seamless upgrades to the main functionality while preserving the state of the tokens and the Synthetix ecosystem.
It should be possible to mitigate malicious front-running of the exchange rate updates by setting a gas price limit for each call to the exchange functionality. This functionality should be facilitated by an exchange gas price limit contract. The Synthetix contract should validate each exchange transaction using this gas price limit contract. It should be possible for the contract owner to adjust the maximum gas price of exchange transactions.
It should be possible for the oracle to disable exchanges during a price update through a price update lock.
Lastly, the oracle should be able to slash the funds of an exchange transaction when it detects extreme cases of front-running. This creates a disincentive for front-runners, making attacks against the system potentially more costly.
The functionality of the Synth token smart contract is described below.
The token should be ERC20 compliant with the following values.
It should be possible to store token balances in an external state contract to allow for seamless upgrades to the main functionality while preserving the state of the tokens and the Synthetix ecosystem.
It should be possible to perform ERC20 transfers with tokens, however, a fee should be reserved on each transfer. It should be possible for either the sender or receiver to pay the fee. Additionally, the token should implement functionality similar to the ERC-223 standard, in that it should trigger a token fallback function in a smart contract that supports the functionality. However, unlike ERC-223, the transfer should not revert if the receiving contract does not support token fallback functionality.
The following section includes in depth descriptions of the findings of the audit.
Synthetix.sol: Line 434-440
The “Protection Circuit” defense mechanism used to protect against front-running was found to be an overly aggressive approach that could lead to loss of user funds. The mechanism operated by allowing the oracle to enable a switch when it detected front-running. When the switch was enabled, any call to the
exchange(...) function would simply burn the tokens that were meant to be exchanged, slashing the front-runner’s funds. The intention was for the oracle to send transactions with high gas prices to enable and disable the switch, only targeting the front-runner’s transactions. However, it would be possible for legitimate users of the system to have their funds slashed due to network interruptions or latency delaying the oracle broadcasting the transaction to disable the protection circuit.
This slashing approach recently gained attention when a front-runner lost their funds due to it. More information can be found in the Synthetix blog post.
SIP7: The oracle contract can disable calls to
exchange(...) while exchange rates are being updated.
The gas price for calls to
exchange(...) must below a maximum threshold set by the oracle. If the price exceeds the limit, the transaction will revert. This mechanism is substantially less aggressive, as it is limited to specific transactions and is less sensitive to network conditions.
As such, it is recommended that the “Protection Circuit” strategy be deprecated from the codebase.
If the slashing approach is still deemed necessary, it is recommended that the functionality be adjusted to at least limit the scope of transactions affected by it. For example, when enabling the switch it would be possible to set a maximum gas price (set at a price that would make front-running infeasible) and only slash transactions above that threshold, protecting ordinary users of the system.
We have migrated Forex price feeds to Chainlink (SIP-32) and soon the rest of the crypto prices will move to Chainlink (SIP-36). Using fee reclamation (SIP-37), described in more detail in Issue-293 will reclaim the risk free profits from front runners. At that stage the protection implementations outlined in SIP-6, SIP-7, and SIP-12 will be deprecated.
No medium risk issues were present at the conclusion of the audit.
SupplySchedule.sol: Line 213
updateMintValues() to establish the number of tokens that could be minted, the function
_remainingSupplyFromPreviousYear(...) was used to calculate whether any tokens were unclaimed from the previous period. Line 218 would then deplete the previous period’s mintable supply. This implementation required
updateMintValues() to be called at least once in a period to accurately track the number of available tokens.
For example, if in period n
updateMintValues() was called, but was not called at least once in period n-1, any unclaimed tokens in n-2 would not be included in the
_remainingSupplyFromPreviousYear(...) calculation and be unaccounted for.
Given that no significant inconsistencies were introduced into the system (e.g. incorrectly tracking the total supply of the token) through this edge-case and that realistically, it is highly unlikely that
updateMintValues() would not be called at least once within a period, no further action is necessarily required.
However, a simple solution would be to iterate through all previous periods and perform the same calculation as on line 195 to determine whether any previous period had remaining tokens available.
A new Inflationary Supply Schedule has since replaced this one in SIP-23 Inflation Smoothing.
The XDR exchange rate was found to spike when adding or removing currencies from the basket. This could result in inconsistencies within the system.
The XDR exchange rate was calculated as the summation of the exchange rates within the basket, e.g.
sUSD = 1, sEUR = 1.25, sAUD = 0.5 XDR = 1 + 1.25 + 0.5 = 2.75
While the XDR currencies were hard-coded in ExchangeRates.sol, the basket could be modified by deleting a rate or through a system upgrade.
Consider the following example - in an early version of the exchange rates smart contract, 1,000 XDRs are issued with an exchange rate of 2.75 (i.e 2,750 USD worth of XDRs in the system). Then an upgrade to the exchange rate contract simply adds sGBP to the basket with an exchange rate of 1.25, and no new tokens are minted. The resulting XDR exchange rate would be 4.0, thus the amount of XDR represented in the system would become worth 4,000 USD, arbitrarily increasing the value by 1,250 USD.
This example highlights the point that it should not be possible to at any stage change the basket given the current method for calculating the XDR exchange rate. This includes adding or removing a currency.
It is recommended that the XDR algorithm be modified to use weighting factors to better accommodate changes in the XDR basket to avoid potential future functional or security issues. However, this would introduce additional complexity into the system for a one-time operation that could pose a substantial risk to the integrity of the system.
XDR’s have been removed from the codebase to be replaced with just using sUSD as the system denominator which is always 1. in SIP-33 Deprecate XDR synth from Synthetix.
The owners of the contracts in the system were found to hold a substantial amount of control over the system. This was a design decision that permits for maintenance, upgradeability, and administration of the system. While this decision might be necessary for a certain stage of the project, it does introduce risk in the form of single points of failure throughout the system. Examples of this include being able to arbitrarily update and control the oracle (e.g. effectively allowing the modification of balances of accounts holding synthetix and synths), and changing the mechanics of the system.
This functionality could be exploited by a rogue actor (e.g. a Synthetix team member with malicious intent) or by a compromised private key and is noted as it extends the potential attack surface of the system. Ideally, the extent of owner capabilities should be limited over time and eventually removed. While the Synthetix team could conspire against their users, it would undermine the integrity of the system and would act against their best interests in ensuring the stability and long-term viability of the network.
As a user of the Synthetix system, it is important to keep in mind that this increases the risk of real-world influence, as a centralised system is more likely to be affected by legislative or political events. Historical examples of this include IDEX and ShapeShift introducing Know Your Customer (KYC) requirements for their users.
The risk of a single rogue actor or a compromised private key can be mitigated by ensuring that a high standard of security is observed by the owner account. For example, the owner account should be a multi-signature wallet, signed by properly secured hardware backed addresses.
As the system matures via regular releases of enhancements and new mechanisims the Synthetix team relinquish control to the community. In order to support the upgradeability of the contracts the owner requires these privileges, once the system is ready these capabilities will be removed and controled by the Synthetix Protocol DAO. Read more about the Transition to decentralised governance on our blog.
Similar to the owner role, the oracle role holds a necessary, substantial influence over the system. This includes operations such as setting gas limits (which if faulty could deny service to the system), slashing account funds through the “Protection Circuit” defense mechanism, and the obvious concern of interpreting off-chain data to track exchange prices.
The oracle code is currently closed source and was excluded from the scope of this audit. It is advised that the Synthetix team perform a comprehensive security audit of the oracle code, and releases a redacted version of the report that reassures users of the system’s integrity without disclosing sensitive information.
We have migrated Forex price feeds to Chainlink in SIP-32 and soon the rest of the crypto prices will move to Chainlink in SIP-36 and the synthetix oracle will be deprecated. Check our blog.synthetix.io for the release roadmap and release announcements.
Actions to improve the functionality and readability of the codebase are outlined below.
_remainingSupplyFromPreviousYear(...) function in SupplySchedule.sol was found to perform an unnecessary check for whether a
uint was less than 0 on lines 198-200. It is recommended that this functionality be removed in order to improve gas performance. Additionally, no other validation is required as a safe
sub(...) function is used to perform the calculation that sets the
It was found that the implicit type
uint was used consistently throughout the project. It is suggested that these
uint type declarations be replaced with the more explicit
uint256 type declaration, where appropriate, in order to adhere to best practice.
While the size of default uint could change between solidity versions, it would be a significant breaking change. At this stage we have quite a few of these around the code base, so rather than changing it, we believe the best approach is to leave it for the time being, changing it to a more specific type if required (e.g. if uint changes in a breaking solidity version).
Version 0.4.25 of the Solidity compiler was used throughout the project. At the time of writing, no publicly available security reviews of the target version were available. A comprehensive security review was performed by the Zeppelin team on version 0.4.24 with several issues being identified and rectified in 0.4.25. As a result, no change is recommended. However, it is worth noting that there is a possibility that new, unknown vulnerabilities exist in the version used.
0.4.25 was prefered over 0.4.24 with the known issues being rectified.
It was found that in several instances, variables were declared with definition names that matched and/or shadowed existing state variables. Although in this case not strictly a security vulnerability or functional issue, it is recommended that these local variables be renamed so as not to mistakenly overshadow any state definitions. Some cases of shadowing identified in the code base are outlined below.
availableCurrencyKeysvariable shadows function name.
ownervariable shadows contract state variable.
It was found that multiple primitive-type variables were uninitialised. Although in this case no functional issues occurred as a result, it is recommended that all numerical-type variables be initialised to zero. These cases are outlined below.
rewardPaidis not initialised.
feesPaidis not initialised.
It was found that multiple mappings and one address variable were using implicitly defined visibility types. Although in this case not strictly a security vulnerability or functional issue, it is recommended that all variables and functions have explicit visibility types specified in order to maintain adherence to coding standards. These cases are outlined below.
FeePool.sol: Line 179
setExchangeFeeRate(uint _exchangeFeeRate) was found to not enforce the maximum exchange fee rate, despite the variable being declared and initialised, and the function comment describing that a limit was enforced. This would allow the owner to set an exchange rate above the maximum exchange rate limit.
require statement should be added to the function to ensure that the
_exchangeFeeRate is below the
MAX_EXCHANGE_FEE_RATE before setting the exchange rate.
Addressed in e13024a.
FeePool.sol: Line 257-258
It was found that although the
setTargetThreshold(...) function evaluated the minimum bound for the
_percent parameter, there was no upper bound restriction applied. This combined with the lack of using the SafeMath library on the arithmetic operation used to set
TARGET_THRESHOLD meant the value would be susceptible to an integer overflow should a large enough value be provided for the
_percent parameter. This results in the validation on line 257 being bypassed entirely.
For instance, if in future the lower bound for
_percent is changed to a value exceeding 0, the validation may be bypassed with an overflow attack allowing
TARGET_THRESHOLD to equal any number. While not strictly a security vulnerability or functional issue, it is suggested that all instances that may result in an integer overflow be attended to in order to prevent future unintended behaviour. This risk can be mitigated by using the SafeMath library for the arithmetic operations used to calculate
Addressed in 5cc3c61.
Certain areas of the codebase were found to have insufficient unit test quality and coverage. It is suggested that unit test code coverage be extended to cover additional functionality. Although no cases could be identified where the lack of coverage would hide a possible security vulnerability or functional issue, it is recommended that these cases be fixed and the test-base be extended to achieve optimal coverage of the cases presented below.
getPenaltyThresholdRatio() added in 6a9f021.
removeInversePricing(...) function in ExchangeRates.sol was found to emit an event regardless of whether a currency was actually removed. A more useful pattern would be to only emit an event if a currency is identified in the for loop.
It might also be helpful to return
true after the event has been emitted, and then revert (or return false) at the end of the loop in the event that no valid currency is found. This would notify the caller to the fact that the function had been called with an invalid currency key.
Addressed in fc730b4.
The inverse pricing mechanism in ExchangeRates.sol was found to be poorly documented within the comments in the codebase. Specifically, the
rateOrInverted(...) function, which was fairly complex, was found to require additional information. This would include details such as the purpose of an inverse price, freezing behaviour, and perhaps an example of an inverse rate being adjusted inversely and ultimately being frozen, much like the example given here.
Addressed in fc730b4.
Interested in getting your smart contracts audited? Request a Quote
This is a limited report on our findings based on our analysis, in accordance with good industry practice as at the date of this report, in relation to: (i) cybersecurity vulnerabilities and issues in the smart contract source code analysed, the details of which are set out in this report, (Source Code); and (ii) the Source Code compiling, deploying and performing the intended functions. In order to get a full view of our findings and the scope of our analysis, it is crucial for you to read the full report. While we have done our best in conducting our analysis and producing this report, it is important to note that you should not rely on this report and cannot claim against us on the basis of what it says or doesn’t say, or how we produced it, and it is important for you to conduct your own independent investigations before making any decisions. We go into more detail on this in the below disclaimer below – please make sure to read it in full.
DISCLAIMER: By reading this report or any part of it, you agree to the terms of this disclaimer. If you do not agree to the terms, then please immediately cease reading this report, and delete and destroy any and all copies of this report downloaded and/or printed by you. This report is provided for information purposes only and on a non-reliance basis, and does not constitute investment advice. No one shall have any right to rely on the report or its contents, and Zenoic Proprietary Limited trading as “Iosiro” and its affiliates (including holding companies, shareholders, subsidiaries, employees, directors, officers and other representatives) (Iosiro) owe no duty of care towards you or any other person, nor does Iosiro make any warranty or representation to any person on the accuracy or completeness of the report. The report is provided "as is", without any conditions, warranties or other terms of any kind except as set out in this disclaimer, and Iosiro hereby excludes all representations, warranties, conditions and other terms (including, without limitation, the warranties implied by law of satisfactory quality, fitness for purpose and the use of reasonable care and skill) which, but for this clause, might have effect in relation to the report. Except and only to the extent that it is prohibited by law, Iosiro hereby excludes all liability and responsibility, and neither you nor any other person shall have any claim against Iosiro, for any amount or kind of loss or damage that may result to you or any other person (including without limitation, any direct, indirect, special, punitive, consequential or pure economic loss or damages, or any loss of income, profits, goodwill, data, contracts, use of money, or business interruption, and whether in delict, tort (including without limitation negligence), contract, breach of statutory duty, misrepresentation (whether innocent or negligent) or otherwise under any claim of any nature whatsoever in any jurisdiction) in any way arising from or connected with this report and the use, inability to use or the results of use of this report, and any reliance on this report.