Infinex Smart Account Smart Contract Audit

</nav>
<article class="report">
<h1 id="introduction">Introduction</h1>
<p>iosiro was commissioned by Infinex to perform a smart contract audit of their Smart Account smart contracts. Three auditors conducted multiple iterative audits from 15 January 2024 to 25 April 2024, with the final audit completed between 21 April 2024 and 25 April 2024, for a total of 25 audit days.</p>
<h4 id="overview">Overview</h4>
<p>Infinex's Smart Account contracts supported on-chain fund deposits and withdrawals and cross-chain functionality. The Smart Account was designed with minimal trust assumptions while providing the account holder full custody over their funds, ensuring that even Infinex cannot access users' funds without explicit authorization.</p>
<p>The codebase underwent significant changes over the course of the audit, mostly due to changes in business requirements. As a result, some earlier findings are not relevant to the current codebase, but have been included for completeness.</p>
<p>The audit uncovered two medium-risk issues, along with several low-risk and informational issues. The first medium-risk issue addressed the uniqueness of signed payloads used to execute sensitive actions in the Smart Account, whereby a signed payload intended for one action could be used to execute a different action.</p>
<p>The second medium-risk issue addressed the security requirements for adding trusted forwarders, a considerably privileged action. The initial security requirements were deemed insufficient and were updated to require Multi-Factor Authentication keys. However, these keys were subsequently removed.</p>
<p>Two low-risk issues remain open. Infinex has indicated that they are working on solutions and will implement them in the future.</p>
<p>The table below shows the reported findings' status at the assessment's conclusion.</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>2</td>
<td>0</td>
</tr>
<tr>
<th>Resolved</th>
<td>0</td>
<td>0</td>
<td>2</td>
<td>4</td>
<td>1</td>
</tr>
<tr>
<th>Closed</th>
<td>0</td>
<td>0</td>
<td>0</td>
<td>1</td>
<td>0</td>
</tr>
</tbody>
</table>
<p>In addition to the reported findings, several code quality improvement suggestions without security implications were provided.</p>
<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> infinex-contracts</li>
<li><strong>Initial audit commit:</strong> <a href="https://github.com/infinex-io/infinex-contracts/tree/bab9768b0b35050b71b74c4e420b70c2a5ba6568">bab9768</a></li>
<li><strong>Final review commit:</strong> <a href="https://github.com/infinex-xyz/infinex-contracts/tree/d267ef612bede48a5ce59a57e31e2ccf416012a8">d267ef6</a></li>
<li><strong>Files:</strong> <code>src/accounts</code>, <code>src/beacons</code>, <code>src/forwarder</code>, <code>src/integrations</code>, <code>src/libraries</code>, <code>src/proxy</code>, <code>Initializable.sol</code></li>
</ul>
<p>A specification of the components included in the audit is provided 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 is aligned with the provided documentation.</li>
</ul>
<p>Off-chain components, such as backend web application code, keeper functionality, and deployment scripts, were explicitly excluded from the 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 considering blockchain technology's experimental and rapidly evolving nature. 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 valuable 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 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 were 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 in the next section of this report.</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-IFX-ACC-001">IO-IFX-ACC-001</a>
</td>
<td>Sudo Key is not purpose bound</td>
<td class="rating-medium">Medium</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-002">IO-IFX-ACC-002</a>
</td>
<td>Insufficient security requirements for adding a trusted forwarder</td>
<td class="rating-medium">Medium</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-003">IO-IFX-ACC-003</a>
</td>
<td>Request should include a deadline</td>
<td class="rating-low">Low</td>
<td class="status-open">Open</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-004">IO-IFX-ACC-004</a>
</td>
<td>Incorrect padding</td>
<td class="rating-low">Low</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-005">IO-IFX-ACC-005</a>
</td>
<td>Unauthenticated bridge functions</td>
<td class="rating-low">Low</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-006">IO-IFX-ACC-006</a>
</td>
<td><code>TrustedForwarder</code> batch execution refund
could be sent to <code>address(0)</code></td>
<td class="rating-low">Low</td>
<td class="status-resolved">Resolved</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-007">IO-IFX-ACC-007</a>
</td>
<td>Wormhole fee not paid</td>
<td class="rating-low">Low</td>
<td class="status-closed">Closed</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-008">IO-IFX-ACC-008</a>
</td>
<td><code>createAccount</code> uses account details that may change as <code>CREATE2</code> salt.</td>
<td class="rating-low">Low</td>
<td class="status-open">Open</td>
</tr>
<tr>
<td>
<a href="#IO-IFX-ACC-009">IO-IFX-ACC-009</a>
</td>
<td><code>circleMinter</code> not verified to be non-zero
by <code>setCircleBridgeParams</code></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, which 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 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 further action.</dd>
</dl>
<p><a name="IO-IFX-ACC-001"></a></p>
<h2 class="break-before" id="io-ifx-acc-001-sudo-key-is-not-purpose-bound"><strong>IO-IFX-ACC-001</strong> Sudo Key is not purpose bound</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/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/accounts/storage/SecurityKeys.sol">SecurityKeys.sol</a>, <a href="https://github.com/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/accounts/utils/RequestTypes.sol">RequestTypes.sol</a></em></td></tr></tbody></table>
<p>With the rework of the security keys and <code>_SIGNATURE_REQUEST_TYPEHASH</code>, the signed requests are no longer sufficiently unique. A suitably positioned attacker can abuse this to perform actions other than that intended by the user.</p>
<p>In the event of a frontend compromise via XSS or supply chain attack, an attacker would be able to hijack authenticated actions like <code>revokeStrongDeviceKey</code> and execute <code>addTrustedForwarder</code> instead (both Requests only contain the <code>_address</code> parameter).</p>
<h3 id="recommendation">Recommendation</h3>
<p>By including the function selector in the <code>Request</code> structure and type hash, the modifier can be updated to validate that the <code>Request</code> corresponds precisely with the intended action.</p>
<pre tabindex="0" class="chroma"><code><span class="line"><span class="cl"><span class="k">if</span> <span class="p">(</span><span class="nb">msg</span><span class="p">.</span><span class="nb">data</span><span class="p">.</span><span class="n">length</span> <span class="o"><</span> <span class="mi">4</span> <span class="o">||</span> <span class="n">_request</span><span class="p">.</span><span class="n">_selector</span> <span class="o">!=</span> <span class="kt">bytes4</span><span class="p">(</span><span class="nb">msg</span><span class="p">.</span><span class="nb">data</span><span class="p">[</span><span class="o">:</span><span class="mi">4</span><span class="p">]))</span> <span class="p">{</span>
</span></span><span class="line"><span class="cl"> <span class="nb">revert</span> <span class="n">Error</span><span class="p">.</span><span class="n">InvalidRequest</span><span class="p">();</span>
</span></span><span class="line"><span class="cl"><span class="p">}</span>
</span></span></code></pre><h3 id="client-response">Client response</h3>
<p>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/0d25193bbe939aab4ca8459881914fcca58a4cee">0d25193</a>.</p>
<p><a name="IO-IFX-ACC-002"></a></p>
<h2 class="break-before" id="io-ifx-acc-002-insufficient-security-requirements-for-adding-a-trusted-forwarder"><strong>IO-IFX-ACC-002</strong> Insufficient security requirements for adding a trusted forwarder</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/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/accounts/modules/BaseModule.sol#L159">BaseModule.sol#L159</a></em></td></tr></tbody></table>
<p>The <code>addTrustedForwarder</code> function in <code>BaseModule</code> can be successfully called with only an Operations Key. An attacker who compromises a user's Operation Key would thus be able to add a trusted forwarder to their Smart Account as a persistence mechanism. This would allow the attacker to relay malicious transactions to the account, which would be trusted even after the compromised Operations Key has been revoked.</p>
<h3 id="recommendation-1">Recommendation</h3>
<p>To reduce the risk of this attack, <code>addTrustedForwarder</code> should be modified to include additional authentication controls, such as requiring an MFA key.</p>
<h3 id="client-response-1">Client response</h3>
<p>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/pull/93/commits/f8db859ae4b4c741b23f8753eff1265b2aa97ad9">f8db859</a>.</p>
<p><a name="IO-IFX-ACC-003"></a></p>
<h2 class="break-before" id="io-ifx-acc-003-request-should-include-a-deadline"><strong>IO-IFX-ACC-003</strong> Request should include a deadline</h2>
<table class="metadata"><tbody><tr><td class="rating-low">Low</td><td class="status-open">Open</td><td><em>General</em></td></tr></tbody></table>
<p>Old or abandoned MFA messages remained valid indefinitely and could be used by an attacker long after being abandoned by a legitimate user.</p>
<h3 id="recommendation-2">Recommendation</h3>
<p>As an in-depth security measure, the <code>Requests</code> signed using the <code>SudoKey</code> should include a <code>deadline</code> to prevent old or abandoned MFA messages from remaining valid. It should then be validated that <code>block.timestamp <= request._deadline</code>.</p>
<h3 id="client-response-2">Client response</h3>
<p>The issue was considered to present a limited risk since the use of signed requests was removed for all functions, excluding <code>addTrustedForwarder</code> and <code>removeTrustedForwarder</code>, by the conclusion of the audit. Regardless, Infinex has indicated that they will include a deadline if the Requests functionality is reintroduced in the future.</p>
<p><a name="IO-IFX-ACC-004"></a></p>
<h2 class="break-before" id="io-ifx-acc-004-incorrect-padding"><strong>IO-IFX-ACC-004</strong> Incorrect padding</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/infinex-io/infinex-contracts/blob/3c996674c097a2a8b28ffaa7965a44269ce52a95/src/integrations/BridgeIntegrations.sol#L106">BridgeIntegrations.sol#L106</a></em></td></tr></tbody></table>
<p>The address used when emitting <code>TokenBridgeMessageRedeemed</code> is incorrect. The token address is left-padded rather than right-padded, so the bit shift is unnecessary.</p>
<p><a href="https://github.com/wormhole-foundation/wormhole-circle-integration/blob/74e11b17ba330640cd2fa72b5a677d6c73165cc1/evm/src/circle_integration/CircleIntegration.sol#L258">https://github.com/wormhole-foundation/wormhole-circle-integration/blob/74e11b17ba330640cd2fa72b5a677d6c73165cc1/evm/src/circle_integration/CircleIntegration.sol#L258</a></p>
<p>The following Tenderly simulation was used to confirm the encoding of the struct:
<a href="https://www.tdly.co/shared/simulation/a86471fd-35f2-4d17-877d-f44f922e2bd0">https://www.tdly.co/shared/simulation/a86471fd-35f2-4d17-877d-f44f922e2bd0</a></p>
<h3 id="recommendation-3">Recommendation</h3>
<p>The bit shift should be removed.</p>
<h3 id="client-response-3">Client response</h3>
<p>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/dd873f7174e2007474beff18965dcc3d3e9d553b">dd873f7</a>.</p>
<p><a name="IO-IFX-ACC-005"></a></p>
<h2 class="break-before" id="io-ifx-acc-005-unauthenticated-bridge-functions"><strong>IO-IFX-ACC-005</strong> Unauthenticated bridge functions</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/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/accounts/modules/BridgingModule.sol#L43-L59">BridgingModule.sol#L43-L59</a></em></td></tr></tbody></table>
<p>The two bridging functions in <code>BridgingModule</code>, <code>bridgeUSDCWithCircle</code> and <code>bridgeUSDCWithWormhole</code>, can be successfully called by any address to bridge the account's balance onto another chain. While there may not be a monetary benefit for an attacker to do this, it could still be done for griefing purposes.</p>
<h3 id="recommendation-4">Recommendation</h3>
<p>To prevent griefing, these bridging functions should only be permitted using a suitable Operations Key.</p>
<h3 id="client-response-4">Client response</h3>
<p>A Sudo or Operations key was required to call the bridging functions when bridging functionality was readded in <a href="https://github.com/infinex-xyz/infinex-contracts/commit/600c79533e3b42f7d30d4c036e55e3a6626b1ece#diff-89b32c0872811f802625ea417d937926a2cd0e753afe5deab8ff900f2b2b00c6">600c795</a>.</p>
<p><a name="IO-IFX-ACC-006"></a></p>
<h2 class="break-before" id="io-ifx-acc-006-trustedforwarder-batch-execution-refund-could-be-sent-to-address0"><strong>IO-IFX-ACC-006</strong> <code>TrustedForwarder</code> batch execution refund could be sent to <code>address(0)</code></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/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/forwarder/InfinexERC2771Forwarder.sol#L173-L198">InfinexERC2771Forwarder.sol#L173-L198</a></em></td></tr></tbody></table>
<p>In <code>InfinexERC2771Forwarder</code>, the <code>_execute</code> function takes two arguments: the request data and a boolean, <code>requireValidRequest</code>. If <code>requireValidRequest</code> is set to <code>true</code>, an invalid nonce or signer will cause the function to revert. If unset, the function will return <code>false</code> without forwarding the request.</p>
<p>The <code>executeBatch</code> function takes an array of requests and a <code>refundReceiver</code> address. If this address is the zero address, it sets <code>requireValidRequest</code> to <code>true</code> when executing requests in the batch. Otherwise, <code>requireValidRequest</code> will be set to <code>false</code>, allowing the function to continue executing requests in the batch even if one has an invalid signature or nonce, and to send any refunded gas to <code>refundReceiver</code>.</p>
<p>However, if all requests in the batch have valid signatures and nonces, but some requests revert, the message value refund could be sent to the zero address. Because of how the <code>refundReceiver</code> parameter is used, users cannot choose to both strictly validate request nonces and signatures and receive a gas refund for failing requests.</p>
<p>The limitations of the function's atomicity are noted in a comment above the function. Still, these limitations could be overcome by having users specify the <code>refundReceiver</code> address and whether to revert on failing signature or nonce verification in separate parameters.</p>
<h3 id="recommendation-5">Recommendation</h3>
<p>Depending on how this functionality is intended to be used, the <code>requireValidRequest</code> boolean can also cause a revert if the request's call fails. In this case, users can freely specify <code>refundReceiver</code> as the zero address, as any reverts caused by requests will also result in the batch request reverting. Therefore, no message value will be lost.</p>
<h3 id="client-response-5">Client response</h3>
<p>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/pull/93/commits/b9c002d11fede6c133ded1054ca30b41b3b5b065">b9c002d</a>. A check was added to revert with <code>ZeroAddress</code> when the <code>refundReceiver</code> is <code>address(0)</code>.</p>
<p><a name="IO-IFX-ACC-007"></a></p>
<h2 class="break-before" id="io-ifx-acc-007-wormhole-fee-not-paid"><strong>IO-IFX-ACC-007</strong> Wormhole fee not paid</h2>
<table class="metadata"><tbody><tr><td class="rating-low">Low</td><td class="status-closed">Closed</td><td><em><a href="https://github.com/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/integrations/BridgeIntegrations.sol#L86">BridgeIntegrations.sol#L86</a></em></td></tr></tbody></table>
<p>When bridging USDC with Wormhole, the Wormhole Circle Bridge function <code>transferTokensWithPayload</code> was called without accounting for the Wormhole bridging fee. At the time of the audit, this fee was set to 0; thus, this finding has been rated as low risk. However, bridging will fail if the fee is set to a non-zero value.</p>
<h3 id="recommendation-6">Recommendation</h3>
<p>The code should be altered to allow the Wormhole bridging fee to be paid.</p>
<h3 id="client-response-6">Client response</h3>
<p>Infinex indicated that they would prefer for transactions to fail in the event of a sudden fee change and are positioned to respond to any upcoming fee changes.</p>
<p><a name="IO-IFX-ACC-008"></a></p>
<h2 class="break-before" id="io-ifx-acc-008-createaccount-uses-account-details-that-may-change-as-create2-salt"><strong>IO-IFX-ACC-008</strong> <code>createAccount</code> uses account details that may change as CREATE2 salt.</h2>
<table class="metadata"><tbody><tr><td class="rating-low">Low</td><td class="status-open">Open</td><td><em><a href="https://github.com/infinex-xyz/infinex-contracts/blob/5c374a4b6c38a029dcf7a55f05ef6df75e64bbc2/src/accounts/AccountFactory.sol#L89">AccountFactory.sol#L89</a></em></td></tr></tbody></table>
<p><code>AccountFactory.createAccount</code> uses CREATE2 with the account's initial Sudo Key as salt to ensure the account has the same address on all compatible chains. As users can add and remove Sudo Keys and have multiple Sudo Keys, there may be a scenario where the original Sudo Key is no longer used for a given account—it may even have been compromised since the account's creation. This could present difficulties for creating the account on new chains or previously unused chains.</p>
<h3 id="recommendation-7">Recommendation</h3>
<p>Off-chain mechanisms should ensure that the same Sudo Key is used for account creation on all chains, even if this Sudo Key is removed from the account. A contingency plan for compromised initial Sudo Keys should also be formulated. For example, users could be encouraged to start new accounts.</p>
<h3 id="client-response-7">Client response</h3>
<p>Infinex is actively investigating potential solutions to this issue.</p>
<p><a name="IO-IFX-ACC-009"></a></p>
<h2 class="break-before" id="io-ifx-acc-009-circleminter-not-verified-to-be-non-zero-by-setcirclebridgeparams"><strong>IO-IFX-ACC-009</strong> <code>circleMinter</code> not verified to be non-zero by <code>setCircleBridgeParams</code></h2>
<table class="metadata"><tbody><tr><td class="rating-informational">Informational</td><td class="status-resolved">Resolved</td><td><em><a href="https://github.com/infinex-io/infinex-contracts/blob/56f2f5db0640a5005b49d43d321dac84758b5956/src/beacons/InfinexProtocolConfigBeacon.sol#L399">InfinexProtocolConfigBeacon.sol#L399</a></em></td></tr></tbody></table>
<p><code>circleMinter</code> is not validated as non-zero when set via <code>InfinexProtocolConfigBeacon.setCircleBridgeParams()</code>. This is inconsistent with the <code>InfinexProtocolConfigBeacon</code> contract's constructor, which does perform this check.</p>
<h3 id="recommendation-8">Recommendation</h3>
<p>To ensure the <code>circleMinter</code> is not accidentally configured to be the zero address, the check below could be added to <code>setCircleBridgeParams()</code>:</p>
<pre tabindex="0" class="chroma"><code><span class="line"><span class="cl"><span class="k">if</span> <span class="p">(</span><span class="n">_circleMinter</span> <span class="o">==</span> <span class="kt">address</span><span class="p">(</span><span class="mi">0</span><span class="p">))</span> <span class="nb">revert</span> <span class="n">Error</span><span class="p">.</span><span class="n">NullAddress</span><span class="p">();</span>
</span></span></code></pre><h3 id="client-response-8">Client response</h3>
<p>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/dcf2be27c0cbd1ce7903af7d232abb2cc78d5de0">dcf2be2</a>.</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/infinex-xyz/infinex-contracts/blob/c1daede0f11f076e6d4294ce7a67e1436142ebca/src/accounts/modules/WithdrawModule.sol">Withdraw<wbr>Module.sol</a></td>
<td>In the current implementation, withdrawals to addresses both on the allowlist and not require the same permission — to be called by the Sudo Key. This makes the allowlist redundant.</td>
</tr>
<tr>
<td>2</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d43b36cfebc982153e2969e61dbfce07164cd4a0/src/accounts/modules/BaseModule.sol#L119">Base<wbr>Module.sol<wbr>#L119</a></td>
<td><code>BaseModule.trustedForwarder()</code> can be renamed to <code>trustedForwarders()</code>, as it returns an array of trusted forwarders.</td>
</tr>
<tr>
<td>3</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d43b36cfebc982153e2969e61dbfce07164cd4a0/src/accounts/modules/AccountUtilsModule.sol#L116">Account<wbr>Utils<wbr>Module.sol<wbr>#L116</a></td>
<td><code>AccountUtilsModule.upgradeProtocolBeaconParameters()</code> does not check if <code>latestInfinexProtocolConfigBeacon</code> is the zero address.</td>
</tr>
<tr>
<td>4</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d43b36cfebc982153e2969e61dbfce07164cd4a0/src/accounts/modules/AccountUtilsModule.sol#L42">Account<wbr>Utils<wbr>Module.sol<wbr>#L42</a></td>
<td><code>AccountUtilsModule.infinexProtocolConfigBeacon()</code> could be made to return <code>Account._infinexProtocolConfig()</code> instead of loading the account storage, to promote code reuse and readability.</td>
</tr>
<tr>
<td>5</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d43b36cfebc982153e2969e61dbfce07164cd4a0/src/accounts/AccountFactory.sol#L60">Account<wbr>Factory.sol<wbr>#L60</a></td>
<td><code>AccountFactory.predictAddress()</code> could be changed to accept the <code>originKey</code> address rather than a <code>bytes32</code> salt, to improve consistency with how the salt is used during creation.</td>
</tr>
<tr>
<td>6</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d43b36cfebc982153e2969e61dbfce07164cd4a0/src/accounts/AccountFactory.sol#L19">Account<wbr>Factory.sol<wbr>#L19</a></td>
<td><code>infinexProtocolConfigBeacon</code> in the <code>AccountFactory</code> contract can be typed <code>IInfinexProtocolConfigBeacon</code>, avoiding the need to cast the <code>infinexProtocolConfigBeacon</code> address every time it is used and thereby improving code readability.</td>
</tr>
<tr>
<td>7</td>
<td>ERC2771<wbr>Context.sol</td>
<td><code>using EnumerableSet for EnumerableSet.AddressSet</code> in <code>ERC2771Context</code> is unnecessary as the library is used explicitly.</td>
</tr>
<tr>
<td>8</td>
<td>ERC2771<wbr>Context.sol</td>
<td><code>ERC2771ContextStorage</code> is unused and can be deleted.</td>
</tr>
<tr>
<td>9</td>
<td>Withdraw<wbr>Module.sol</td>
<td>To improve readability, named function parameters should be used when passing members of the <code>Request</code> struct. e.g. <code>_withdrawUSDCToChain({_amount: _request._uint256, _addressTo: _request._address, _destinationDomain: _request._uint32});</code></td>
</tr>
<tr>
<td>10</td>
<td>Bridging<wbr>Module.sol</td>
<td>Function state mutability can be restricted to view for <code>_getBridgeAmount</code> and <code>_calculateGasFeesOwed</code></td>
</tr>
<tr>
<td>11</td>
<td>Bridging<wbr>Module.sol</td>
<td><code>Account._infinexProtocolConfig().depositBridgingActive()</code> can be refactored to an internal function <code>_checkDepositBridgeActive()</code>. Improves readability and avoids code duplication.</td>
</tr>
<tr>
<td>12</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/accounts/modules/WithdrawModule.sol#L153">Withdraw<wbr>Module.sol<wbr>#L153</a></td>
<td>The first line in <code>withdrawToAllowlistedAddress</code> is considered redundant and can be removed.</td>
</tr>
<tr>
<td>13</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/accounts/modules/WithdrawModule.sol#L291">Withdraw<wbr>Module.sol<wbr>#L291</a></td>
<td>The fee cap in <code>WithdrawalModule._takeWithdrawalFee</code> is hardcoded as 50. This could be stored in a constant to improve code readability.</td>
</tr>
<tr>
<td>14</td>
<td><a href="https://github.com/infinex-io/infinex-contracts/blob/d53e5377bbabb9fa1fa8c278d28a7c2f92f5dafb/src/beacons/InfinexProtocolConfigBeacon.sol#L317-L421">Infinex<wbr>Protocol<wbr>Config<wbr>Beacon.sol<wbr>#L317-L421</a></td>
<td><code>InfinexProtocolConfigBeacon</code> contains several administrative setter functions. Events are emitted by the setter functions, which alter protocol fees, but not by setter functions, which make other changes to the protocol, such as setting the addresses of different pools, implementations and bridges. Adding events to these functions would enhance the protocol's transparency and monitorability.</td>
</tr>
</tbody>
</table>
<ol>
<li>Per Infinex, this represents the protocol's current security posture and is subject to change in future.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/214daf4770ba4d638f16c5bf7b8fc1ec4d2522b9">214daf4</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/5a3b6c954f58414caf6227971a8d32504bf190b3">5a3b6c9</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/d350ff188f253cf28838c863a5055e22d4df904c">d350ff1</a>.</li>
<li>Will not fix.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/043b2f39a4fee2dc24a277e90a65fd3fc210bc00">043b2f3</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/c12bac94ed8097cbd442755b856bd74773c07b56">c12bac9</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/c12bac94ed8097cbd442755b856bd74773c07b56">c12bac9</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/c12bac94ed8097cbd442755b856bd74773c07b56">c12bac9</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/c12bac94ed8097cbd442755b856bd74773c07b56">c12bac9</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/c12bac94ed8097cbd442755b856bd74773c07b56">c12bac9</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/e9ac0744eead774b904dea6f9660d6df3568b5f2">e9ac074</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/e9ac0744eead774b904dea6f9660d6df3568b5f2">e9ac074</a>.</li>
<li>Fixed in <a href="https://github.com/infinex-io/infinex-contracts/commit/e9ac0744eead774b904dea6f9660d6df3568b5f2">e9ac074</a>.</li>
</ol>
<h1 id="specification">Specification</h1>
<p>The following section outlines the system's intended functionality at a high level, based on its 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="account-types">Account types</h2>
<p>Smart Accounts are divided into deposit accounts and trading accounts, which are modular smart contracts developed using <a href="https://sips.synthetix.io/sips/sip-307/">Synthetix's Router Proxy Architecture</a>.</p>
<p>Deposit accounts will be deployed to local and foreign chains. Funds can be deposited to these accounts and bridged to trading accounts on other chains using Wormhole and CCTP. Deposits are made in USDC and used to pay Infinex fees during fund withdrawals.</p>
<p>Trading accounts will exist on chains, such as Base, where trading will happen. The codebase did not yet include any trading functionality, but initiating fund withdrawals to local and foreign chains was possible.</p>
<h2 id="account-creation">Account creation</h2>
<p>Accounts are created using the <code>AccountFactory</code> contract, which accepts an initial Sudo Key as part of the salt when deploying a new Smart Account contract. As such, Smart Accounts can be deterministically created across multiple EVM-based chains through CREATE2.</p>
<p>The Sudo Key supplied during account creation is the primary authorization mechanism for sensitive user actions. Additional Sudo Keys can be added to a single account, and existing Sudo Keys can be removed once the account is created.</p>
<h2 id="security-keys">Security keys</h2>
<p>Each account can have a variety of security keys. These are addresses used to initiate different account actions. The security key schema underwent significant changes between the start and completion of the audit, with the final schema being as follows:</p>
<dl>
<dt>Sudo Keys</dt>
<dd>As mentioned above, these are the main authorization mechanisms for user actions on accounts. They were previously called Origin keys.</dd>
<dt>Operation keys</dt>
<dd>These are lower-privileged keys used for bridging funds between chains. They were previously called Device keys.</dd>
<dt>Recovery keys</dt>
<dd>These are addresses used for emergency withdrawals (detailed below).</dd>
</dl>
<p>Each account can have one or more Sudo Keys and zero or more s and recovery keys. The address of a valid Sudo Key must be used when adding or removing any key.</p>
<h2 id="smart-account-settings-and-implementation-updates">Smart Account settings and implementation updates</h2>
<p>The Smart Account allows users to opt into Infinex updates to their accounts' settings and implementations, which could enhance users' control over their accounts. If Infinex were to update the Smart Account implementation to a version users are not yet comfortable adopting, they can postpone updating their Smart Accounts for as long as they wish. The same applies to general account settings, such as the Wormhole bridge parameters, Circle bridge parameters, and the Infinex configuration beacon address.</p>
<p>It should be noted that users can only update their Smart Accounts to the latest configuration parameters as provided by the Infinex configuration beacon. Upgrading their Smart Account implementations to arbitrary implementations is prohibited.</p>
<h2 id="deposits-and-withdrawals">Deposits and withdrawals</h2>
<p>Deposits to the trading account are made by transferring USDC directly to the trading account if depositing from the same chain or depositing to a deposit account on a foreign chain and triggering a bridge transfer. Depending on source chain availability, either Wormhole or CCTP can be used to bridge. Additionally, the bridging module supports bridging USDC to a Solana trading account via CCTP.</p>
<p>Withdrawal of ERC-20, ERC-721 and native tokens to the local chain are also supported. Withdrawals of ERC-20 and native tokens incur a USDC fee set in the Infinex configuration beacon's reported fee but with a hardcoded cap of $50 in the Smart Account code. Allowlisted withdrawal addresses are also configurable, which is a functionality that Infinex have indicated will be expanded upon in the future.</p>
<h2 id="recovery-module">Recovery module</h2>
<p>The recovery module allows users to configure recovery addresses to which funds can be withdrawn in the event of a security incident. Adding a recovery address allows Infinex to take action on behalf of users in emergencies while ensuring users have full custody of their funds.</p>

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

‍

Secure your system.

Request a service

Start Now →