Synthetix Aspidiske Release Smart Contract Audit
iosiro was commissioned by Synthetix to conduct a smart contract audit of their Aspidiske Release, which included SIP-252
The audit of SIP-252 was performed across the following periods:
A total of 29 resource days were used to audit across this period.
This report is organized into the following sections.
The information in this report should be used to understand the smart contracts' risk exposure better and as a guide to improving the security posture of the smart contracts by remediating issues identified. The results of this audit reflect the in-scope source code reviewed at the time of the audit.
The purpose of this audit was to achieve the following:
Assessing the off-chain functionality associated with the contracts, for example, backend web application code, was outside of 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 high risk from 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. Strategies that should be used to encourage secure code development include:
This report presents the findings of a smart contract audit performed by iosiro of Synthetix's Aspidiske release.
SIP-252 introduced a mechanism to allow the liquidation of escrowed SNX in the system when a position's collateralization ratio is below the liquidation ratio threshold. The addition of escrow entry liquidations allows all collateral held by a staker to be liquidated, which further improves the system's ability to maintain its health. Escrow entries can only be liquidated by forced (i.e. non-self) liquidations if the transferrable SNX in a position is insufficient to increase their collateralization ratio to the requisite amount. Furthermore, the liquidator and flagger fee mechanism applied during forced liquidations was updated to improve the calculations of penalizations and in some cases close accounts that were previously left open.
Two high risk issues relating to accounts with Synthetix Escrow and flag removal logic were identified, as well as one medium risk and various other low risk and informational issues. At the end of the audit, the code was found to be of a high standard and conformed to the specification provided.
The source code considered in-scope for the assessment is described below. Code from all other files was considered to be out-of-scope. Out-of-scope code that interacts with in-scope code was assumed to function as intended and not introduce any functional or security vulnerabilities for the purposes of this audit.
Project Name: Synthetix
Initial commit: 1678d32
Intermediary commit: 73e7463
Final commit: 159a5da
Files: BaseRewardEscrowV2.sol, ImportableRewardEscrowV2.sol, BaseSynthetix.sol, Issuer.sol, Liquidator.sol, RewardEscrowV2.sol, RewardEscrowV2Storage.sol
A variety of techniques were used in order to perform 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 test environment, both manually and through the test suite provided. Manual analysis was used to confirm that the code was functional and to identify security issues that could be exploited.
Tools were used to automatically detect the presence of several types of security vulnerabilities, including reentrancy, timestamp dependency bugs, and transaction-ordering dependency bugs. Static analysis results were reviewed manually and any false positives were removed. Any true positive results are included in this report.
Static analysis tools commonly used include Slither, Securify, and MythX. Tools such as the Remix IDE, compilation output, and linters could also be 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 specification of SIP-252 was based on commit hash 10fae03.
The following section includes in-depth descriptions of the findings of the audit.
No high-risk issues identified during the audit were present at the conclusion of the audit.
No medium-risk issues identified during the audit were present at the conclusion of the audit.
No low-risk issues identified during the audit were present at the conclusion of the audit.
No informational issues identified during the audit were present at the conclusion of the audit.
The implementation neglects that a user could have SNX in the SynthetixEscrow contributing to their c-ratio. When paying out liquidator rewards, the transaction could fail, as the necessary amount of SNX is not liquid. As a result, accounts that fall into this category could be prevented from being liquidated. This issue is also present in the containing if statement.Issuer.sol#L779. Furthermore, liquidating an account like this could result in all of its debt being removed despite having additional funds available. One solution would be to perform a partial liquidation in this case, e.g. calculate how much debt was actually paid off and then return that. The problem is only relevant to L1 as L2 does not have escrowed entries in this contract. Around 723,907 SNX is currently unclaimed.
The _collateral function should be updated to no longer include the Synthetix Escrow balance held by accounts as a form of collateral.
The suggestion was implemented in f4d754c.
As partial liquidations are possible with self-liquidation (and potentially forced liquidations if the change is implemented), the collateralization ratio of the position should be checked before removing the flag. The risk with this issue is that a user could continuously remove any liquidation flag set on their account by self-liquidating any SNX position (i.e. a non-zero value of debtToRemove) that will then reset the flag timer, preventing the account from being liquidated.
Flag removal should be limited to only forced liquidations.
The suggestion was implemented in bdad565.
A liquidation will revert if the liquidated account has exactly enough collateral to pay the liquidator rewards, e.g. _collateral(account) = (rewardsSum) Issuer.sol#L788, or when liquidating an account with all of its collateral in the rewards escrow.
The revert should be replaced by an if statement around the liquidator rewards transfer logic and call to notifyRewardAmount.
The suggestion was implemented in 2fdcddf.
Using balanceOf instead of the _collateral function results in self-liquidations overcorrecting if there are escrowed entries.
The function should use the _collateral function to ensure that the target collateralization ratio is reached after a self-liquidation.
The suggestion was implemented in f4d754c.
When the liquidated account has a liquidatable collateral amount equal to the sum of the liquidator and flagger fees, the liquidated collateral is paid as staker rewards and not to the flagger and liquidator. This is not in line with the SIP-252 specification, which states that in such a case, the collateral should be paid to the liquidator and flagger.
The totalRedeemed > flagReward.add(liquidateReward) condition should rather use >= to ensure that the liquidator and flagger fees are paid when their sum is equal to the redeemed collateral amount.
This issue was resolved in c2dc94a.
The liquidationPenalty() helper in Liquidator.sol uses getLiquidationPenalty() to load the penalty value from FlexibleStorage.sol which returns a different value than getSnxLiquidationPenalty() used by Issuer.sol when performing a liquidation. The helper function is not used during the liquidation process, so this difference has no risk to the system.
The liquidationPenalty() helper function should be updated to use the getSnxLiquidationPenalty() function instead to ensure the correct value is provided to callers attempting to perform liquidation calculations off-chain.
This issue was resolved in 9bdc006.
