Nexus Mutual USD Price Feed Oracle Support Audit

<defs>

</g>

</symbol>

</defs>

</svg>

</svg>

</div>

</svg>

<div class="frontpage-title">USD Price Feed Oracle Support Audit</div>

<div class="frontpage-subtitle">Nexus Mutual, 07 November 2024</div>

<h1>Contents</h1>

<ul>

<li>

<a href="#introduction">Introduction</a></li>

<li>

<a href="#disclaimer">Disclaimer</a></li>

<li>

<a href="#methodology">Methodology</a></li>

<li>

<a href="#audit-findings">Audit findings</a><ul>

<li>

<a href="#io-nxm-upo-001-verify-oracle-price-decimal-value">IO-NXM-UPO-001 Verify oracle price decimal value</a></li>

<li>

<a href="#io-nxm-upo-002-verify-asset-decimals-is-set">IO-NXM-UPO-002 Verify asset decimals is set</a></li>

</ul>

</li>

<li>

<a href="#specification">Specification</a><ul>

<li>

<a href="#usd-price-oracle-changes">USD price oracle changes</a></li>

</ul>

</li>

</ul>

</nav>

<h1 id="introduction">Introduction</h1>

<p>iosiro was commissioned by Nexus Mutual to perform a smart contract audit of changes introduced to the Nexus Mutual protocol to support chainlink USD price feed oracles. The audit took place between the 31st of October and the 7th of November 2024, using 2 audit days.</p>

<h4 id="overview">Overview</h4>

<thead>

<tr>

<th>Critical</th>

<th>Medium</th>

<th>Informational</th>

</tr>

</thead>

<tbody>

<tr>

</tr>

<tr>

<td>Resolved</td>

</tr>

<tr>

<td>Closed</td>

</tr>

</tbody>

</table>

<h4 id="scope">Scope</h4>

<p>The assessment focused on source file 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>Initial audit commit:</strong> <a href="https://github.com/NexusMutual/smart-contracts/tree/ca1ad5d533674553ad3c3acf333eedfd6a415a1e">ca1ad5d</a></li>

<li><strong>Final review commit:</strong> <a href="https://github.com/NexusMutual/smart-contracts/tree/575781b0e6bf2f43712e7fe308df7fae90b26c80">575781b</a></li>

<li><strong>Files:</strong> <code>IPriceFeedOracle.sol</code> <code>PriceFeedOracle.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>

<thead>

<tr>

<th>Issue</th>

<th>Status</th>

</tr>

</thead>

<tbody>

<tr>

<td>Verify oracle price decimal value</td>

<td class="status-resolved">Resolved</td>

</tr>

<tr>

<td>Verify asset decimals is set</td>

<td class="rating-informational">Informational</td>

<td class="status-resolved">Resolved</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>

<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>

<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>

<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-UPO-001"></a><h2 id="io-nxm-upo-001-verify-oracle-price-decimal-value" class="break-before"><strong>IO-NXM-UPO-001</strong> Verify oracle price decimal value</h2>

<tbody>

<tr>

<td class="status-resolved">Resolved</td>

<td><a href="https://github.com/NexusMutual/smart-contracts/blob/ca1ad5d533674553ad3c3acf333eedfd6a415a1e/contracts/modules/capital/PriceFeedOracle.sol#L104-L120">PriceFeedOracle.sol#L104-L120</a></td>

</tr>

</tbody>

</table>

<p>The decimals of the ETH and USD price oracles should be validated to ensure that they are as expected, i.e. 18 decimals for ETH and 8 decimals for USD. Some USD price oracles, such as <a href="https://etherscan.io/address/0xe20CA8D7546932360e37E9D72c1a47334af57706#readContract">AMPL/USD</a>, use 18 decimals instead of 8 decimals. If AMPL were to be introduced as an asset type, the price returned by the oracle would be calculated incorrectly.</p>

<h3 id="recommendation">Recommendation</h3>

<p>Verification of the price decimal value can be done in the constructor. Alternatively, if Nexus Mutual wish to support any price decimal values, then the <code>PriceFeedOracle::_getAssetToEthRate()</code> function would need to be modified to appropriately scale the returned price, based on the returned USD and/or ETH price decimal.</p>

<h3 id="client-response">Client response</h3>

<p>Fixed in <a href="https://github.com/NexusMutual/smart-contracts/commit/575781b0e6bf2f43712e7fe308df7fae90b26c80">575781b</a>.</p>

<a name="IO-NXM-UPO-002"></a><h2 id="io-nxm-upo-002-verify-asset-decimals-is-set" class="break-before"><strong>IO-NXM-UPO-002</strong> Verify asset decimals is set</h2>

<tbody>

<tr>

<td class="rating-informational">Informational</td>

<td class="status-resolved">Resolved</td>

<td><a href="https://github.com/NexusMutual/smart-contracts/blob/ca1ad5d533674553ad3c3acf333eedfd6a415a1e/contracts/modules/capital/PriceFeedOracle.sol#L33-L42">PriceFeedOracle.sol#L33-L42</a></td>

</tr>

</tbody>

</table>

<p>When constructing a new <code>PriceFeedOracle</code> contract, it should be verified that the decimal value passed into the constructor for each asset is set to a value greater than zero. Doing so will prevent setting an incorrect value, resulting in an incorrect price being returned by the various getter functions in this contract</p>

<h3 id="recommendation-1">Recommendation</h3>

<p>Verify that the asset decimal value is non-zero in the <code>PriceFeedOracle</code> constructor.</p>

<h3 id="client-response-1">Client response</h3>

<p>Fixed in <a href="https://github.com/NexusMutual/smart-contracts/commit/05e40cdecfcd5b98d20b440164a3dfd45a7479b3">05e40cd</a>.</p>

<h1 id="specification">Specification</h1>

<p>The following section outlines the changes made to the protocol at a high level, based on their implementations 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="usd-price-oracle-changes">USD price oracle changes</h2>

<p>Nexus Mutual added support for USD chainlink price feed oracles to their protocol, enabling support for assets that do not have ETH chainlink oracles, such as cbBTC. The <code>PriceFeedOracle</code> contract was modified to introduce an <code>AggregatorType</code> enum to distinguish between USD and ETH price feeds. The type was verified against the expected price decimal in the constructor and ensured that the array of asset oracles included an ETH/USD price oracle. The internal <code>PriceFeedOracle::_getAssetToEthRate()</code> function was also modified to calculate the equivalent ETH rate based on the asset and USD prices for those assets that did not have an ETH price oracle.</p>

</article>

</main>

</div>

‍