Nexus Mutual Safe Tracker Smart Contract Audit

</nav>
<article class="report">
<h1 id="introduction">Introduction</h1>
<p>iosiro was commissioned by Nexus Mutual to conduct a smart contract audit of their Safe Tracker. The audit was performed by 2 auditors between 23 and 24 January 2024, using 3 audit days.</p>
<h4 id="overview">Overview</h4>
<p>This report presents the findings of an audit performed by iosiro of Nexus Mutual's Safe Tracker smart contracts. iosiro identified two medium risk issues, as well as one low risk issue and several informational issues. The two medium issues could have resulted in:</p>
<ul>
<li>A malicious Safe having the ability to arbitrarily adjust the reported value of the Safe Tracker.</li>
<li>A malicious Safe front-running an approval transaction from the Controller, approving an unexpected token type and amount of tokens for transfer.</li>
</ul>
<p>Overall, the code was found to be of a high standard. The changes covered by the audit were relatively small and straightforward. Both unit tests and fork tests were provided with the code changes.</p>
<p>The table below shows the status of reported findings at the conclusion of the assessment.</p>
<table class="findings-count">
<thead>
<tr>
<th></th>
<th>Critical</th>
<th>High</th>
<th>Medium</th>
<th>Low</th>
<th>Informational</th>
</tr>
</thead>
<tbody>
<tr>
<th>Open</th>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
</tr>
<tr>
<th>Resolved</th>
<td>0</td>
<td>0</td>
<td>2</td>
<td>1</td>
<td>0</td>
</tr>
<tr>
<th>Closed</th>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>3</td>
</tr>
</tbody>
</table>
<hr />
<h4 id="scope">Scope</h4>
<p>The assessment focused on 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> <code>smart-contracts</code></li>
<li><strong>Commits:</strong> <a href="https://github.com/nexusmutual/smart-contracts/tree/47580f2731b968e3804cb7e2a3b754d06162e1a3">47580f2</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/a3e4806111deeaa07e407cc40db1882b33b57da4">a3e4806</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/9b406fb335af175eace2c4bd2611d6b4a798286e">9b406fb</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/aa38334c7826e941a87b88c634c39a8338f7f35f">aa38334</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/740815f6ca30cfbf1b30208777c2952524a0694f">740815f</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/a9eca851bac777fb9140303e0da9ccd3ab16a662">a9eca85</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/5674767c315ef5859daab7c96783092b3a4752a9">5674767</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/3fef214e86d36a2418c07151998dbfc6a074a27c">3fef214</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/189b493e647f3d7c68f000ba27dc35c6d6d27ed3">189b493</a>, <a href="https://github.com/nexusmutual/smart-contracts/pull/1058/commits/a63ee34c4ff5997bed05ad17e7ebdd3e7996b2e3">a63ee34</a></li>
<li><strong>Files:</strong> <code>PriceFeedOracle.sol</code>, <code>SafeTracker.sol</code>, <code>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 aligned with the provided 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 which takes into account 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-SFT-001">IO-NXM-SFT-001</a>
</td>
<td>Missing validation of Cover Re investment amount</td>
<td class="rating-medium">Medium</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-NXM-SFT-002">IO-NXM-SFT-002</a>
</td>
<td>Potential for front-running to manipulate requested amount</td>
<td class="rating-medium">Medium</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-NXM-SFT-003">IO-NXM-SFT-003</a>
</td>
<td>Potential underflow in asset value calculation</td>
<td class="rating-low">Low</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-NXM-SFT-004">IO-NXM-SFT-004</a>
</td>
<td>Potential for setting of <code>MasterAware</code> contract to be front-run</td>
<td class="rating-informational">Informational</td>
<td class="status-closed">Closed</td>
</tr>
<tr>
<td>
<a href="#IO-NXM-SFT-005">IO-NXM-SFT-005</a>
</td>
<td>No staleness check of Chainlink values</td>
<td class="rating-informational">Informational</td>
<td class="status-closed">Closed</td>
</tr>
<tr>
<td>
<a href="#IO-NXM-SFT-006">IO-NXM-SFT-006</a>
</td>
<td>Lack of outflow limit</td>
<td class="rating-informational">Informational</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 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>
<p><a name="IO-NXM-SFT-001"></a></p>
<h2 class="break-before" id="io-nxm-sft-001-missing-validation-of-cover-re-investment-amount"><strong>IO-NXM-SFT-001</strong> Missing validation of Cover Re investment amount</h2>
<table class="metadata"><tbody><tr><td class="rating-medium">Medium</td><td class="status-resolved">Resolved</td><td><em><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L57">SafeTracker.sol#L57</a></em></td></tr></tbody></table>
<p>The<code>updateCoverReInvestmentUSDC</code> function allows the Safe to arbitrarily set the value of <code>coverReInvestmentUSDC</code>, which in turn impacts the total value of the Safe Tracker and thereby the book value of NXM in the RAMM. As Safe members have a vested interest in the NXM exchange rate, it is advisable to validate that this amount is within a range deemed to be acceptable beforehand.</p>
<h3 id="recommendation">Recommendation</h3>
<p>As the value of the Cover Re investment cannot be determined on-chain without an Oracle system, a simpler control to put in place would be to hardcode an absolute limit for the investment value. While this approach would theoretically still allow for manipulation of the value, it would limit the potential impact of doing so.</p>
<h3 id="client-response">Client response</h3>
<p>A limit was implemented in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/a3e4806111deeaa07e407cc40db1882b33b57da4">a3e4806</a>.</p>
<p><a name="IO-NXM-SFT-002"></a></p>
<h2 class="break-before" id="io-nxm-sft-002-potential-for-front-running-to-manipulate-requested-amount"><strong>IO-NXM-SFT-002</strong> Potential for front-running to manipulate requested amount</h2>
<table class="metadata"><tbody><tr><td class="rating-medium">Medium</td><td class="status-resolved">Resolved</td><td><em><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SwapOperator.sol#L551">SwapOperator.sol#L551</a></em></td></tr></tbody></table>
<p>The<code>transferRequestedAsset</code> function relies on a previously stored <code>transferRequest</code>. This pattern makes it possible for the Safe to either intentionally or unintentionally front-run an approval by the controller, resulting in the approval of an unexpected token type and amount.</p>
<h3 id="recommendation-1">Recommendation</h3>
<p>The <code>transferRequestedAsset</code> function should require the caller to pass in the expected values for validation.</p>
<h3 id="client-response-1">Client response</h3>
<p>The recommendation was implemented in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/9b406fb335af175eace2c4bd2611d6b4a798286e">9b406fb</a>.</p>
<p><a name="IO-NXM-SFT-003"></a></p>
<h2 class="break-before" id="io-nxm-sft-003-potential-underflow-in-asset-value-calcuation"><strong>IO-NXM-SFT-003</strong> Potential underflow in asset value calcuation</h2>
<table class="metadata"><tbody><tr><td class="rating-low">Low</td><td class="status-resolved">Resolved</td><td><em><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L117">SafeTracker.sol#L117</a></em></td></tr></tbody></table>
<p>The order of operations in the final line of <code>_calculateBalance</code> could lead to an underflow in some circumstances, causing the function to revert.</p>
<h3 id="recommendation-2">Recommendation</h3>
<p>In the <code>_calculateBalance</code> function, subtraction of debt should be done after all the positive values have been summed to avoid the potential for an underflow that may result in the function reverting unnecessarily.</p>
<h3 id="client-response-2">Client response</h3>
<p>The recommendation was implemented in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/aa38334c7826e941a87b88c634c39a8338f7f35f">aa38334</a>.</p>
<p><a name="IO-NXM-SFT-004"></a></p>
<h2 class="break-before" id="io-nxm-sft-004-potential-for-setting-of-masteraware-contract-to-be-front-run"><strong>IO-NXM-SFT-004</strong> Potential for setting of <code>MasterAware</code> contract to be front-run</h2>
<table class="metadata"><tbody><tr><td class="rating-informational">Informational</td><td class="status-closed">Closed</td><td><em><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/abstract/MasterAwareV2.sol#L79">MasterAwareV2.sol#L79</a></em></td></tr></tbody></table>
<p>The MasterAware pattern is susceptible to front-running as the master is set separately from the contract deployment. It is advisable to set the master address in the constructor.</p>
<p>However, the Safe contract is intended to be deployed as a proxy, so the master address will be set atomically during its deployment. In this scenario, this finding poses no risk.</p>
<h3 id="client-response-3">Client response</h3>
<p>The team noted the issue and accepted the risk.</p>
<p><a name="IO-NXM-SFT-005"></a></p>
<h2 class="break-before" id="io-nxm-sft-005-no-staleness-check-of-chainlink-values"><strong>IO-NXM-SFT-005</strong> No staleness check of Chainlink values</h2>
<table class="metadata"><tbody><tr><td class="rating-informational">Informational</td><td class="status-closed">Closed</td><td><em><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/PriceFeedOracle.sol#L70">PriceFeedOracle.sol#L70</a></em></td></tr></tbody></table>
<p>The recency of the Chainlink value can be used to determine whether the value is stale and revert if the price is too old.</p>
<h3 id="client-response-4">Client response</h3>
<p>The team noted the issue and accepted the risk.</p>
<p><a name="IO-NXM-SFT-006"></a></p>
<h2 class="break-before" id="io-nxm-sft-006-lack-of-outflow-limit"><strong>IO-NXM-SFT-006</strong> Lack of outflow limit</h2>
<table class="metadata"><tbody><tr><td class="rating-informational">Informational</td><td class="status-closed">Closed</td><td><em><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol">SafeTracker.sol</a></em></td></tr></tbody></table>
<p>Currently there is no mechanism for controlling the net outflow of assets from the Pool to the Safe. The Safe and Swap Controllers could theoretically collude to steal the entirety of the funds of the Pool. It may be desirable to limit the gross outflow of assets to a given amount that can be adjusted by another role, such as <code>onlyAuthorizedToGovern</code>.</p>
<p>Due to trust assumptions elsewhere in Nexus Mutual’s protocol, if these actors were to collude they would be able to push through malicious governance proposals or use other means to achieve similar goals. As throttling fund flows in this context might impact the Safe’s ability to manage its debt positions, adding such a control would likely only serve to increase the risk faced by the system.</p>
<h3 id="client-response-5">Client response</h3>
<p>The team noted the issue and accepted the risk.</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/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L86">Safe<wbr>Tracker.sol<wbr>#L86</a>, <a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L90">Safe<wbr>Tracker.sol<wbr>#L90</a></td>
<td>A linter should be used to ensure consistent code formatting to improve readability. An example is the inconsistency of including or excluding a space between the return parameters and the start of the function scope (e.g. <code>){</code> and <code>) {</code>).</td>
</tr>
<tr>
<td>2</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L48">Safe<wbr>Tracker.sol<wbr>#L48</a>, <a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L79">Safe<wbr>Tracker.sol<wbr>#L79</a></td>
<td>The Pool address is accessed using inconsistent methods throughout the code. It should either be consistently referenced as <code>address(pool()))</code> or <code>internalContracts[uint(ID.P1)]</code> for code clarity.</td>
</tr>
<tr>
<td>3</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L57">Safe<wbr>Tracker.sol<wbr>#L57</a></td>
<td>The <code>_investedUSDC</code> parameter in the <code>updateCoverReInvestmentUSDC</code> function was preceded by an underscore, but this pattern was not used elsewhere in the codebase.</td>
</tr>
<tr>
<td>4</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/PriceFeedOracle.sol#L25">Price<wbr>Feed<wbr>Oracle.sol<wbr>#L25</a>, <a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L27">Safe<wbr>Tracker.sol<wbr>#L27</a></td>
<td>PriceFeedOracle constructor could validate that the SafeTracker is a non-zero address. Additionally, <code>safe</code>, <code>usdc</code>, <code>dai</code>, <code>aweth</code>, <code>debtUsdc</code> can be validated to be non-zero in SafeTracker’s constructor.</td>
</tr>
<tr>
<td>5</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L57">Safe<wbr>Tracker.sol<wbr>#L57</a></td>
<td>The <code>updateCoverReInvestmentUSDC</code> function should emit an event with the amount of USDC invested.</td>
</tr>
<tr>
<td>6</td>
<td><a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L67">Safe<wbr>Tracker.sol<wbr>#L67</a>, <a href="https://github.com/NexusMutual/smart-contracts/blob/47580f2731b968e3804cb7e2a3b754d06162e1a3/contracts/modules/capital/SafeTracker.sol#L78">Safe<wbr>Tracker.sol<wbr>#L78</a></td>
<td>An internal <code>_transfer</code> function can be used to reduce code duplication in <code>transfer</code> and <code>transferFrom</code>.</td>
</tr>
</tbody>
</table>
<ol>
<li>The formatting issues were addressed in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/740815f6ca30cfbf1b30208777c2952524a0694f">740815f</a>.</li>
<li>The recommendation was implemented in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/a9eca851bac777fb9140303e0da9ccd3ab16a662">a9eca85</a>.</li>
<li>The preceding underscore was removed in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/5674767c315ef5859daab7c96783092b3a4752a9">5674767</a>.</li>
<li>Validation was added in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/3fef214e86d36a2418c07151998dbfc6a074a27c">3fef214</a>.</li>
<li>The event was added in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/189b493e647f3d7c68f000ba27dc35c6d6d27ed3">189b493</a>.</li>
<li>The recommendation was implemented in <a href="https://github.com/NexusMutual/smart-contracts/pull/1058/commits/a63ee34c4ff5997bed05ad17e7ebdd3e7996b2e3">a63ee34</a>.</li>
</ol>
<h1 id="specification">Specification</h1>
<p>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.</p>
<h2 id="safe">Safe</h2>
<p>This is a Gnosis Safe controlled by three of five Advisory Board members. It is able to receive ETH from the Pool in order to perform various investment related operations, such as managing Aave V3 loans and set the amount of the Cover Re USDC investment.</p>
<h2 id="safe-tracker">Safe Tracker</h2>
<p>The Safe Tracker is a new contract in the system that follows the ERC-20 standard. The balance of the Safe and the total supply are calculated as the total value of the assets in terms of ETH.</p>
<p>The calculation to determine the total value is as follows (all values are denominated in ETH terms):</p>
<pre><code>ETH balance of the Safe +
Aave Wrapped ETH balance of the Safe +
USDC balance of the Safe +
Amount allocated to the Cover Re (as specified by the Pool) -
Aave USDC debt value
</code></pre>
<p>The USDC to ETH conversion rate is based on the exchange rate reported by Chainlink. Any further assets in the pool are excluded from the calculation.</p>
<p>The Safe Tracker is upgradeable through a proxy.</p>
<h2 id="swap-operator">Swap Operator</h2>
<p>The existing Safe Tracker was adapted to allow the Safe to initiate a two-stage process to transfer assets from the Pool to the Safe, as follows:</p>
<ol>
<li>The Safe calls the Swap Operator indicating a token type and amount requested from the Pool.</li>
<li>The Controller then calls the Swap Operator accepting the transfer request, thereby actioning the transfer of assets from the Pool to the Safe.</li>
</ol>

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

‍