dHEDGE Synthetix V3 Integration Smart Contract Audit
# 1. Introduction
iosiro was commissioned by dHEDGE to conduct a smart contract audit of their Synthetix V3 Integration. The audit was performed by 2 auditors between 4 and 15 December 2023, using 20 resource days. A final review of changes was performed between 24 and 26 January 2024, using 5 resource days.
This report is organized into the following sections.
- **[Section 2 - Executive summary:](#section-2)** A high-level description of the findings of the audit.
- **[Section 3 - Audit details:](#section-3)** A description of the scope and methodology of the audit.
- **[Section 4 - Design specification:](#section-4)** An outline of the intended functionality of the smart contracts.
- **[Section 5 - Open findings:](#section-5)** Detailed descriptions of audit findings that remained present at the conclusion of the audit.
- **[Section 6 - Closed findings:](#section-6)** Detailed descriptions of audit findings that were addressed during the audit.
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 the 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:
- Identify potential security flaws.
- Ensure that the smart contracts function according to the documentation provided.
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:
- Security should be integrated into the development lifecycle, and the level of perceived security should not be limited to a single code audit.
- Defensive programming should be employed to account for unforeseen circumstances.
- Current best practices should be followed where possible.
<a name="section-2"></a>
# 2. Executive summary
This report presents the findings of an audit performed by iosiro on dHEDGE's Synthetix V3 integration.
### Audit findings
iosiro identified two low risk issues, and one informational issue. The first low risk issue could result in a temporary Denial of Service (DoS) of a fund manager’s account, whereas the second low risk issue made it possible for fund managers to deny fund participants the ability to withdraw their assets. The informational issue was related to access control on functions that emitted protocol events, which could cause some grievance. The final review revealed that dHedge addressed all the issues identified.
In addition to reviewing the issues identified during the original audit, iosiro also reviewed a decimal conversion bug independently identified by the dHEDGE team.
### Recommendations
At a high level, the security posture of dHEDGE’s Synthetix V3 integration could be further strengthened by:
- Remediating the issues identified in this report and performing a review to ensure that the issues were correctly addressed.
- Performing additional audits at regular intervals, as security best practices, tools, and knowledge change over time. Additional audits throughout the project's lifespan ensure the longevity of the codebase.
<a name="section-3"></a>
# 3. Audit details
## 3.1 Scope
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.
### 3.1.1 Smart contracts
- **Project name:** dhedge-v2
- **Commits:** [a7bc402](https://github.com/dhedge/dhedge-v2/tree/a7bc4027ba545633ce06187168446feea085fd37)
- **Final Commit**: [f2c1bc1](https://github.com/dhedge/dhedge-v2/tree/f2c1bc19509acb53fc7d44bffc5f79c741a1f7d7)
- **Files:** `SynthetixV3ContractGuard.sol`, `SynthetixV3SpotMarketContractGuard.sol`, `SynthetixV3AssetGuard.sol`, `WeeklyWindowsHelper.sol`
## 3.2 Methodology
The audit was conducted using a variety of techniques described below.
### 3.2.1 Code review
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.
### 3.2.2 Dynamic analysis
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 discover security issues that could be exploited.
### 3.2.3 Automated analysis
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.
## 3.3 Summary of findings
The table below provides an overview of the audit's findings. Detailed write-ups are provided in [Section 5](#section-5) and [Section 6](#section-6).
<table><thead><tr><th>#</th><th>Issue</th><th>Risk</th><th>Status</th></tr></thead><tbody><tr><td><a href="#issue-6-3-1">6.3.1</a></td><td>Transferring an Account NFT to the pool before one is created results in temporary DoS</td><td class="risk-low">Low</td><td class="status-closed">Closed</td></tr><tr><td><a href="#issue-6-3-2">6.3.2</a></td><td>Fund manager could prevent fund participants from being able to withdraw</td><td class="risk-low">Low</td><td class="status-closed">Closed</td></tr><tr><td><a href="#issue-6-4-1">6.4.1</a></td><td>Guard contract logs poisonable</td><td class="risk-informational">Informational</td><td class="status-closed">Closed</td></tr></tbody></table>
Each issue identified during the audit has been assigned a risk rating. The rating is determined based on the criteria outlined below.
- **High risk**: The issue could result in a loss of funds for the contract owner or system users.
- **Medium risk**: The issue resulted in the code specification being implemented incorrectly.
- **Low risk**: A best practice or design issue that could affect the security of the contract.
- **Informational**: A lapse in best practice or a suboptimal design pattern that has a minimal risk of affecting the security of the contract.
In addition to a risk rating, each issue is assigned a status:
- **Open**: The issue remained present in the code as of the final commit reviewed and may still pose a risk.
- **Closed**: The issue was identified during the audit and has since been satisfactorily addressed, removing the risk it posed.
<a name="section-4"></a>
# 4. Design specification
The following section outlines the intended functionality of the system at a high level. This specification is based on the implementation in the codebase. Any perceived points of conflict should be highlighted with the auditing team to determine the source of the discrepancy.
The Synthetix V3 integration for dHedge will allow fund managers to use investor funds to support Synthetix markets, earning investors fees from Synthetix V3. The goal of the new integration is to support the Synthetix V3 perps market by supplying liquidity. Fund managers will be allowed to interact with investor funds though the `PoolLogic.sol` contract, which makes use of the Synthetix V3 guards. The guards ensure that fund managers can’t behave in a malicious manner, and ensure that only fund managers and designated traders can interact with investor funds.
Investors will be able to deposit USDC, sUSDC or sUSD. sUSDC, however, will be unwrapped to USDC when investors wish to withdraw from the fund.
## Contract Guard
`SynthetixV3ContractGuard.sol`, which implements the contract guard, is responsible for allowing fund managers to perform a reduced number of actions against Synthetix V3 Core. The actions include creating a V3 account, depositing and withdrawing collateral, delegating collateral, and minting and burning sUSD. The primary validation logic relates to the account the fund manager supplies, as well as the collateral types to be managed.
The contract guard is designed such that fund managers do not have direct control over the V3 account, but can perform various actions as the account.
## Spot Market Contract Guard
The Spot Market contract guard, implemented in `SynthetixV3SpotMarketContractGuard.sol`, allows fund managers to buy and sell, and wrap and unwrap, on the Synthetix V3 sUSDC Spot Market. However, this is restricted to assets that don’t incur fees as the goal is to only convert investor collaterals to and from usable tokens. The Spot Market contract guard makes use of a list of “allowed markets” which determine which assets the fund manager is allowed to wrap to, unwrap from, buy and sell.
## Asset Guard
The Asset guard, implemented in `SynthetixV3AssetGuard.sol`, is responsible for facilitating investor withdrawals from the fund. The Asset guard calculates the amount a specific investor is owed when they wish to exit the fund, and ensures that there are funds available to reimburse the investor. It determines whether any of the withdrawal assets need to be unwrapped, specifically sUSDC to USDC, and generates transaction data that the `PoolLogic.sol` contract will execute.
The amounts that investors can withdraw is primarily subject to the amount of collateral available less any debt incurred by the delegated position. Under normal conditions the delegated position would earn more rewards than debt incurred.
<a name="section-5"></a>
# 5. Open findings
The following section details the findings of the audit which remained open at the conclusion of the final review.
## 5.1 High Risk
No high-risk issues were open at the conclusion of the audit.
## 5.2 Medium Risk
No medium-risk issues were open at the conclusion of the audit.
## 5.3 Low risk
No low-risk issues were open at the conclusion of the audit.
## 5.4 Informational
No informational issues were open at the conclusion of the audit.
<a name="section-6"></a>
# 6. Closed findings
The following section details findings of the audit which were addressed during the audit.
## 6.1 High risk
No high-risk issues were identified during the audit.
## 6.2 Medium risk
No medium-risk issues were identified during the audit.
## 6.3 Low risk
<a name="issue-6-3-1"></a>
### 6.3.1 Transferring an Account NFT to the pool before one is created results in temporary DoS
[*SynthetixV3ContractGuard.sol#128*](https://github.com/dhedge/dhedge-v2/blob/a7bc4027ba545633ce06187168446feea085fd37/contracts/guards/contractGuards/synthetixV3/SynthetixV3ContractGuard.sol#L128), [*SynthetixV3ContractGuard.sol#268*](https://github.com/dhedge/dhedge-v2/blob/a7bc4027ba545633ce06187168446feea085fd37/contracts/guards/contractGuards/synthetixV3/SynthetixV3ContractGuard.sol#L268)
#### Description
As shown in the SynthetixV3 integration test “NFT Account and its permissions → should revert when trying to create Synthetix V3 NFT account if pool already has received one through airdrop”, attempts to execute `createAccount()` or `createAccount(uint128)` will revert if an Account NFT is transferred using `transferFrom()` to the pool before either create is called (this does not work with `safeTransferFrom()` as it calls the receive hook which invokes `SynthetixV3ContractGuard.verifyERC721()`). This is caused by the ERC721 balance checks in `txGuard()` ([SynthetixV3ContractGuard.sol#128](https://github.com/dhedge/dhedge-v2/blob/a7bc4027ba545633ce06187168446feea085fd37/contracts/guards/contractGuards/synthetixV3/SynthetixV3ContractGuard.sol#L128)) and `_afterTxGuardHelper` ([SynthetixV3ContractGuard.sol#268](https://github.com/dhedge/dhedge-v2/blob/a7bc4027ba545633ce06187168446feea085fd37/contracts/guards/contractGuards/synthetixV3/SynthetixV3ContractGuard.sol#L268)), which will revert if the pool contract already holds one or more Account NFTs. As Account NFTs transferred to the contract in this way will not be automatically tracked by `DhedgeNftTrackerStorage`, this will render SynthetixV3 functionality unavailable. It would, however, be possible for the owner of `DhedgeNftTrackerStorage` to manually register an Account NFT owned by the pool by calling `addDataByUintId()`, making this a temporary rather than permanent denial of service.
The test below demonstrates the bug and this mitigation. It can be inserted into the “NFT Account and its permissions” section of the SynthetixV3 integration tests.
<pre class="language-javascript">
<code>
it("should use airdropped NFT account", async () => {
// manually create a Synthetix V3 account NFT and transfer it to pool
let dropid = await ownerCreatesPositionAndTransfersItToPool();
// NFT was transferred in but is not registered, so NFT ID is 0
expect(await infrastructureData.synthetixV3ContractGuard.getAccountNftTokenId(poolAddress, synthetixV3CoreAddress)).to.equal(0);
// Create account reverts because pool already owns 1 AccountNFT
await expect(createAccount()).to.be.revertedWith("only one account allowed");
// owner of dhedgeNftTrackerStorage registers airdropped NFT to pool
await infrastructureData.dhedgeNftTrackerStorage.connect(deployments.owner).addDataByUintId(
SYNTHETIX_ACCOUNT_NFT_TYPE,
poolAddress,
dropid
);
// now pool can use the airdropped NFT
expect(await infrastructureData.synthetixV3ContractGuard.getAccountNftTokenId(poolAddress, synthetixV3CoreAddress)).to.equal(dropid);
});
</code>
</pre>
#### Recommendation
To avoid a situation where a pool’s SynthetixV3 functionality requires admin intervention to re-enable, the above-mentioned balance checks in `txGuard` and `_afterTxGuardHelper` could be replaced with calls to `DhedgeNftTrackerStorage.getDataCount()`, which would return the number of NFTs currently being tracked. This would prevent a new account NFT from being created for the pool if one is already in use, regardless of how many account tokens are held by the pool. Note that this change would invalidate the assumption made by the token ID retrieval in `afterTxGuard()` . Instead of retrieving the ID of the pool’s first token, it would be necessary to retrieve the ID of the pool’s most recent token.
#### Update
This issue was remediated by verifying the `PoolLogic` contract’s NFT balance through the `DhedgeNftTrackerStorage`. The code changes were implemented in [466191f](https://github.com/dhedge/dhedge-v2/commit/466191fe3d099eaaa305c15951720ba5d8f13554).
<a name="issue-6-3-2"></a>
### 6.3.2 Fund manager could prevent fund participants from being able to withdraw
[*SynthetixV3AssetGuard.sol#L156*](https://github.com/dhedge/dhedge-v2/blob/61ad57f84f7d9cee44dcfdd01a3815c7f8a875cc/contracts/guards/assetGuards/synthetixV3/SynthetixV3AssetGuard.sol#L156)
#### Description
The fund does not undelegate collateral from its position in Synthetix V3 when fund participants withdraw from the fund. Under normal conditions a fund manager would keep some collateral undelegated in the fund; however, in the case where a fund manager were to delegate all of the deposited collateral fund participants would be unable to withdraw. A malicious fund manager could keep all user funds delegated in perpetuity and, using a multicall contract, they could undelegate and withdraw profits from fees earned without freeing collateral for other users to withdraw.
#### Recommendation
The fund could implement delegation and undelegation windows. During the delegation window the fund manager is free to delegate all collateral to Synthetix V3 and during the undelegation window withdrawal requests from the fund are able to undelegate the required collateral to complete the withdrawal. Due to the delegation period in Synthetix V3 only one undelegation would be allowed per Synthetix delegation period. This delegation period could be managed through the use of withdrawal queues, where users wishing to exit from the fund commit a withdrawal request, and during the undelegation window one single undelegation of the total queued withdrawal amount would be submitted to Synthetix V3 (provided there are no undelegated funds available).
#### Update
This issue was remediated by introducing windows that can only be used for either delegating or undelegating collateral. Fund managers can delegate collateral only when in the delegation window, whereas any fund participant can trigger undelegation if the Synthetix V3 pool does not have sufficient free collateral to satisfy the fund participant’s withdrawal. The code changes were implemented in [ef007b5](https://github.com/dhedge/dhedge-v2/commit/ef007b5b0299106a1e6589a131b22795e8f474b5).
## 6.4 Informational
<a name="issue-6-4-1"></a>
### 6.4.1 Guard contract logs poisonable
[*SynthetixV3ContractGuard.sol#L106*](https://github.com/dhedge/dhedge-v2/blob/a7bc4027ba545633ce06187168446feea085fd37/contracts/guards/contractGuards/synthetixV3/SynthetixV3ContractGuard.sol#L106)
#### Description
The `txGuard` functions in `SynthetixV3ContractGuard` and `SynthetixV3SpotMarketContractGuard` (as well as other guards in the codebase) can be called by any contract or EOA. If valid transaction data is provided with these calls, the contract will emit a `SynthetixV3Event` with the provided pool address and transaction type. While the transaction would not actually be executed and no state would be altered, this does allow for poisoning of the guard contract’s event logs.
This is demonstrated by the test below, which can be inserted into the ”NFT Account and its permissions” section of the SynthetixV3 integration tests.
<pre class="language-javascript">
<code>
it("should emit event when txGuard called with valid data by an unprivileged user", async () => {
await expect(
infrastructureData.synthetixV3ContractGuard
.connect(deployments.user)
.txGuard(whitelistedManagerLogic.address, synthetixV3CoreAddress, IAccountModule.encodeFunctionData("createAccount()", []))
).to.emit(infrastructureData.synthetixV3ContractGuard, "SynthetixV3Event")
});
</code>
</pre>
#### Recommendation
To prevent this, execution of `txGuard` could be restricted to the pool in the same manner as `afterTxGuard`.
#### Update
This issue was remediated by requiring that `txGuard`, in both the `SynthetixV3ContractGuard` and `SynthetixV3SpotMarketContractGuard` contracts, be called by a whitelisted `poolLogic` contract. The code changes were implemented in [6f8e078](https://github.com/dhedge/dhedge-v2/commit/6f8e0781ec0655b63206183af891d359181f61ed)**.**
