Nexus Mutual Long Term Limit Order Smart Contract Audit

</nav>
<article class="report">
<h1 id="introduction">Introduction</h1>
<p>iosiro was commissioned by Nexus Mutual to perform an audit of changes made to their <code>Pool</code> and <code>SwapOperator</code> contracts to better track the value of assets in the <code>Pool</code> contract as well as increase the maximum possible validity period of swap orders. The audit was conducted between the 16th and the 18th of September 2024, using 3 audit days.</p>
<h4 id="overview">Overview</h4>
<p>The audit uncovered one low-risk issue which was subsequently closed. This issue could lead to inaccurate pool value when the pool is upgraded while there was an ongoing swap order; however, Nexus Mutual have indicated that they will ensure that there are no ongoing swap orders when upgrading the <code>Pool</code> contract. Additionally, one code quality suggestion was made, as detailed in this report.</p>
<table class="findings-count">
<thead>
<tr>
<th> </th>
<th>Critical</th>
<th>High</th>
<th>Medium</th>
<th>Low</th>
</tr>
</thead>
<tbody>
<tr>
<td>Open</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
</tr>
<tr>
<td>Resolved</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
</tr>
<tr>
<td>Closed</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>1</td>
</tr>
</tbody>
</table>
<h4 id="scope">Scope</h4>
<p>The assessment focused on the changes made to the source files listed below, with all other files considered out of scope. Any out-of-scope code interacting with the assessed code was presumed to operate correctly without introducing functional or security vulnerabilities.</p>
<ul>
<li><strong>Project name:</strong> <a href="https://github.com/NexusMutual/smart-contracts/tree/audit/long-term-limit-order">NexusMutual/smart-contracts/tree/audit/long-term-limit-order</a></li>
<li><strong>Initial audit commit:</strong> <a href="https://github.com/NexusMutual/smart-contracts/commit/70847c57d0f9c1735b8645a54d6c96530d6347ef">70847c5</a></li>
<li><strong>Final review commit:</strong> <a href="https://github.com/NexusMutual/smart-contracts/tree/c5021ce211d66f9f8bf6bd472ac2a5d038b64d8a">c5021ce</a></li>
<li><strong>Files:</strong> <code>contracts/modules/capital/Pool.sol</code>, <code>contracts/modules/capital/SwapOperator.sol</code>,</li>
</ul>
<p>A specification is available in the <a href="#specification">Specification section</a> of this report.</p>
<h1 id="disclaimer">Disclaimer</h1>
<p>This report aims to provide an overview of the assessed smart contracts' risk exposure and a guide to improving their security posture by addressing identified issues. The audit, limited to specific source code at the time of review, sought to:</p>
<ul>
<li>Identify potential security flaws.</li>
<li>Verify that the smart contracts' functionality aligns with their documentation.</li>
</ul>
<p>Off-chain components, such as backend web application code, keeper functionality, and deployment scripts were explicitly not in-scope of this audit.</p>
<p>Given the unregulated nature and ease of cryptocurrency transfers, operations involving these assets face a high risk from cyber attacks. Maintaining the highest security level is crucial, necessitating a proactive and adaptive approach that accounts for the experimental and rapidly evolving nature of blockchain technology. To encourage secure code development, developers should:</p>
<ul>
<li>Integrate security throughout the development lifecycle.</li>
<li>Employ defensive programming to mitigate the risks posed by unexpected events.</li>
<li>Adhere to current best practices wherever possible.</li>
</ul>
<h1 id="methodology">Methodology</h1>
<p>The audit was conducted using the techniques described below.</p>
<dl>
<dt>Code review</dt>
<dd>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.</dd>
<dt>Dynamic analysis</dt>
<dd>The contracts were compiled, deployed, and tested in a test environment, both manually and through the test suite provided. Dynamic analysis was used to identify additional edge cases, confirm that the code was functional, and to validate the reported issues.</dd>
<dt>Automated analysis</dt>
<dd>Automated tooling was used to detect the presence of various types of security vulnerabilities. Static analysis results were reviewed manually and any false positives were removed. Any true positive results are included in this report.</dd>
</dl>
<h1 id="audit-findings">Audit findings</h1>
<p>The table below provides an overview of the audit's findings. Detailed write-ups are provided below.</p>
<table class="findings">
<thead>
<tr>
<th>ID</th>
<th>Issue</th>
<th>Risk</th>
<th>Status</th>
</tr>
</thead>
<tbody>
<tr>
<td><a href="#IO-NXM-LTO-001">IO-NXM-LTO-001</a></td>
<td>SwapOperator data not copied on construction</td>
<td class="rating-low">Low</td>
<td class="status-closed">Closed</td>
</tr>
</tbody>
</table>
<p>Each issue identified during the audit has been assigned a risk rating. The rating is determined based on the criteria outlined below.</p>
<dl>
<dt>Critical risk</dt>
<dd>The issue could result in the theft of funds from the contract or its users.</dd>
<dt>High risk</dt>
<dd>The issue could result in the loss of funds for the contract owner or its users.</dd>
<dt>Medium risk</dt>
<dd>The issue resulted in the code being dysfunctional or the specification being implemented incorrectly.</dd>
<dt>Low risk</dt>
<dd>A design or best practice issue that could affect the ordinary functioning of the contract.</dd>
<dt>Informational</dt>
<dd>An improvement related to best practice or a suboptimal design pattern.</dd>
</dl>
<p>In addition to a risk rating, each issue is assigned a status:</p>
<dl>
<dt>Open</dt>
<dd>The issue remained present in the code as of the final commit reviewed and may still pose a risk.</dd>
<dt>Resolved</dt>
<dd>The issue was identified during the audit and has since been satisfactorily addressed, removing the risk it posed.</dd>
<dt>Closed</dt>
<dd>The issue was identified during the audit and acknowledged by the developers as an acceptable risk without actioning any change.</dd>
</dl>
<a name="IO-NXM-LTO-001"></a><h2 id="io-nxm-lto-001-swapoperator-data-not-copied-on-construction" class="break-before"><strong>IO-NXM-LTO-001</strong> SwapOperator data not copied on construction</h2>
<table class="metadata">

<tbody>
<tr>
<td class="rating-low">Low</td>
<td class="status-closed">Closed</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/70847c57d0f9c1735b8645a54d6c96530d6347ef/contracts/modules/capital/Pool.sol#L62">Pool.sol#L62</a></td>
</tr>
</tbody>
</table>
<p>When a new <code>Pool</code> contract is created, the <code>assetsInSwapOperatorBitmap</code> value and the <code>assetsInSwapOperator</code> mapping values are not copied from the legacy pool in the constructor. As a result, if there was an ongoing swap order, the pool’s value would not include the order’s related assets in the <code>SwapOperator</code> contract.</p>
<h3 id="recommendation">Recommendation</h3>
<p>The <code>assetsInSwapOperatorBitmap</code> and <code>assetsInSwapOperator</code> values should be copied from the legacy pool in the constructor. This will require changing the visibility of <code>assetsInSwapOperatorBitmap</code> from <code>internal</code> to <code>public</code> .</p>
<h3 id="client-response">Client response</h3>
<p>Nexus Mutual will always close any swap orders and transfer assets from the <code>SwapOperator</code> contract back into the <code>Pool</code> contract before performing any upgrades on the <code>Pool</code> contract.</p>
<h1 id="code-quality-improvement-suggestions">Code quality improvement suggestions</h1>
<p>Code improvement suggestions without security implications are listed below.</p>
<table class="codequality">
<thead>
<tr>
<th>#</th>
<th>Location</th>
<th>Details</th>
</tr>
</thead>
<tbody>
<tr>
<td>1</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/70847c57d0f9c1735b8645a54d6c96530d6347ef/contracts/modules/capital/Pool.sol#L34">Pool.sol#L34</a></td>
<td>A <code>uint</code> value could be used instead of the existing <code>assetsInSwapOperator</code> mapping to track the amount of assets in the <code>SwapOperator</code> contract. This is because at most only a single order and consequently only one asset can be in the <code>SwapOperator</code> at any given time.</td>
</tr>
</tbody>
</table>
<p>The above improvement suggestion was implemented in commit <a href="https://github.com/NexusMutual/smart-contracts/commit/9ba01325fccc793a95a0c2fc0ebfd4788e92c147">9ba0132</a>.</p>
<h1 id="specification">Specification</h1>
<p>The following section outlines the main functional changes to the system at a high level as implemented in the codebase. Any perceived points of conflict should be highlighted with the auditing team to determine the source of the discrepancy.</p>
<h2 id="pool-contract-changes">Pool contract changes</h2>
<p>To allow better tracking of the pool value during an ongoing swap, changes were made mainly to the <code>Pool</code> contract. Specifically, the method for tracking the value of assets in the <code>SwapOperator</code> contract was changed to track the amounts of assets transferred to that contract instead of the value of those assets at the time of the transfer. This was coupled with changes to the <code>Pool::getPoolValueInEth()</code> and <code>Pool::getAssetValueInEth()</code> functions to take into account in asset amounts in the <code>SwapOperator</code> when calculating the pool value given the current asset price. Additionally, <code>Pool::setSwapAssetAmount()</code> replaced <code>Pool::setSwapValue()</code>, allowing <code>SwapOperator</code> to set the asset amounts for an open order in a mapping -- <code>assetsInSwapOperator</code> -- and a corresponding bitmap -- <code>assetsInSwapOperatorBitmap</code>. The bitmap provided a gas-efficient way of determining whether an asset was in the <code>SwapOperator</code> contract.</p>
<p>While this change improved the accuracy of the pool value, it may still be inaccurate in cases where an order has been partially or fully filled, as the asset amounts in the <code>SwapOperator</code> would change. This change would not be reflected in the pool's value until the order is closed and all assets are transferred back to the pool.</p>
<h2 id="swapoperator-contract-changes">SwapOperator contract changes</h2>
<p>The maximum validity period for an order was increased from 60 minutes to 31 days. Additionally, the contract was modified to use the new <code>Pool::SetSwapAmountInSwapOperator()</code> when executing an asset transfer for a swap order.</p>

</article>
</main>
</div>

‍