Infinex Accounts V4

</nav>
<article class="report">
<h1 id="introduction">Introduction</h1>
<p>iosiro was commissioned by Infinex to perform a smart contract audit of the Infinex Accounts V4 release. A single auditor conducted the audit between 20 November 2024 and 3 December 2024, using 3 audit days.</p>
<h4 id="overview">Overview</h4>
<p>During the audit, one low-risk issue related to the incorrect implementation of EIP-712 and one informational issue relating to the lack of validation for specific requests were reported. Several code quality improvements and defence-in-depth suggestions were also provided and are detailed in this report.</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>
<td>Open</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>1</td>
</tr>
<tr>
<td>Resolved</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>1</td>
<td>0</td>
</tr>
<tr>
<td>Closed</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
<td>0</td>
</tr>
</tbody>
</table>
<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>infinex-contracts</code></li>
<li><strong>Initial audit commit:</strong> <a href="https://github.com/infinex-xyz/infinex-contracts/tree/42253373a0fec113f9fad879ff0fa1ece99a0892"><code>4225337</code></a></li>
<li><strong>Final review commit:</strong> <a href="https://github.com/infinex-xyz/infinex-contracts/tree/0e142681656af4854ca5eeef5421da5612230b92"><code>0e14268</code></a></li>
</ul>
<table>
<thead>
<tr>
<th>File</th>
<th>SHA256 - <a href="https://github.com/infinex-xyz/infinex-contracts/tree/0e142681656af4854ca5eeef5421da5612230b92"><code>0e14268</code></a></th>
</tr>
</thead>
<tbody>
<tr>
<td width="55%">src/accounts/utils/RequestTypes.sol</td>
<td>47f7d9e0aef9cb95b439fdff5d01c232528650d5f62d29c86f4fcb67135f8c42</td>
</tr>
<tr>
<td>src/accounts/utils/SecurityModifiers.sol</td>
<td>7c5a6dde338befc3983cf13fecf088e24347f05f431602ca7e3f08cf91c03305</td>
</tr>
<tr>
<td>src/accounts/utils/WithdrawUtil.sol</td>
<td>f7a4e0576bb7045e87c005ec27808d141edefe189b40032c41e1365f928628e0</td>
</tr>
<tr>
<td>src/accounts/utils/AccountConstants.sol</td>
<td>e6f4420118e4b841d54a2e31f65af8905c86f01068bba4270efdca2f867c0e98</td>
</tr>
<tr>
<td>src/accounts/utils/SecurityUtils.sol</td>
<td>f5a3c07dde9a117ee23c5fc129e81f3a591ce934f29854e7c639202eae5e81f0</td>
</tr>
<tr>
<td>src/accounts/storage/App.sol</td>
<td>b2373c34fd674b0f31d0e509c1b5765a6f662adafd9f0407ad0ed86d26fad8df</td>
</tr>
<tr>
<td>src/accounts/storage/Recovery.sol</td>
<td>40dbef83e013a53d3e4948e486ccc18d03b21bb83a55737558a4692d11906176</td>
</tr>
<tr>
<td>src/accounts/storage/Cosigning.sol</td>
<td>c0ef1b04c501b381fe4e1f57ea8bc998e8209a7c9a749caded17a49690c76d57</td>
</tr>
<tr>
<td>src/accounts/storage/Account.sol</td>
<td>246704d480ac812c1d729dc68345dc415d80bc36e8572cbded606fe28324c21a</td>
</tr>
<tr>
<td>src/accounts/storage/Bridge.sol</td>
<td>cc8f253b656a46d605b53e1e19664448d4560d412032913383641d7861abe188</td>
</tr>
<tr>
<td>src/accounts/storage/Withdrawal.sol</td>
<td>4ebd51230bf0a903b7dfb8fedd840f32b73693c3a83ea09908ca45203a29546e</td>
</tr>
<tr>
<td>src/accounts/storage/SecurityKeys.sol</td>
<td>b4177aa5ae5d56cac6c05bbbfea50211b6acc2c95325c235cb18e90cdbfe30fb</td>
</tr>
<tr>
<td>src/accounts/storage/EIP712.sol</td>
<td>4823614863254c55d2fc7e63b9887686cca6ac8fc14c7ac8a32aaca7e264d1ab</td>
</tr>
<tr>
<td>src/accounts/modules/WithdrawModule.sol</td>
<td>999e0d7c2bbd3a6de6431c6bcd47158eb73eaf3d0b3b01aada10ce8f16c1e2ee</td>
</tr>
<tr>
<td>src/accounts/modules/BaseModule.sol</td>
<td>1e42e73a9ffd4515bd49e97be7beaab1748045bf75ac874596fa7db411c7c9fb</td>
</tr>
<tr>
<td>src/accounts/modules/RecoveryModule.sol</td>
<td>49b9d78223e29786a16f37c80a909842826c0ef8d8a78b71b6d7c8545178265f</td>
</tr>
<tr>
<td>src/accounts/modules/AccountUtilsModule.sol</td>
<td>248198167e03918429fc72254fa229238903cc97d83695fb0cd07b7a8ad94514</td>
</tr>
<tr>
<td>src/accounts/modules/AppModule.sol</td>
<td>a2c9794154fcbde5b9f14c8be3b1bb888a0e786e2bd1d579331605271a87d5b7</td>
</tr>
<tr>
<td>src/accounts/modules/TransferModule.sol</td>
<td>7e6849ae05bf0e8c219d58771d4e59906448b51d88bb2b0999295442677dccb4</td>
</tr>
<tr>
<td>src/accounts/modules/BridgingModule.sol</td>
<td>334f4582588276809d3968ebd13d78b31c8f03f9b2ebfdd2720245cdcec124ad</td>
</tr>
<tr>
<td>src/beacons/InfinexProtocolConfigBeacon.sol</td>
<td>51bde80280667d5a1ef27446d60c3e8002d2927bc63da5528374f825bb3fbbc7</td>
</tr>
<tr>
<td width="55%">src/libraries/NonStandardToken.sol</td>
<td>6deddd61fca03af57adf52840568bcf50f77cc5b4704af9dd23ceed1b556bcbf</td>
</tr>
</tbody>
</table>
<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-IFX-RVIV-001">IO-IFX-RVIV-001</a></td>
<td>Missing validation</td>
<td class="rating-informational">Informational</td>
<td class="status-open">Open</td>
</tr>
<tr>
<td><a href="#IO-IFX-RVIV-005">IO-IFX-RVIV-005</a></td>
<td>Type hash does not conform to EIP-712</td>
<td class="rating-low">Low</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>
<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-IFX-RVIV-001"></a><h2 id="io-ifx-rviv-001-missing-validation" class="break-before"><strong>IO-IFX-RVIV-001</strong> Missing validation</h2>
<table class="metadata">

<tbody>
<tr>
<td class="rating-informational">Informational</td>
<td class="status-open">Open</td>
<td><a href="https://github.com/infinex-xyz/infinex-contracts/blob/d08bde067095c70c8ad03458311a118ceb7576f8/src/accounts/storage/Cosigning.sol#L12">Cosigning.sol#L12</a></td>
</tr>
</tbody>
</table>
<p>The validation of transfer request properties is inconsistent and differs between conceptually identical functions.</p>
<p>Infinex uses passkeys to sign transactions, which does not provide users with a clear indication of the data being signed. Therefore, any errors in how structs are encoded could result in transfer requests containing zero-valued parameters. In the worst case, if the recipient parameter was misaligned, funds could accidentally be transferred to the zero address.</p>
<h3 id="recommendation">Recommendation</h3>
<p>Validation checks should be added for some or all relevant parameters of the following functions:</p>
<ul>
<li><code>_transferERC20</code>: The zero address should not be permitted as the <code>destination</code> or <code>token</code> parameters.</li>
<li><code>_transferERC1155</code>: None of the transfer request's properties are validated. Only non-zero values for <code>destination</code>, <code>token</code> and <code>amount</code> should be permitted.</li>
<li><code>_transferERC1155Batch</code>: None of the transfer request's properties are validated. Only non-zero values for the <code>destination</code> and <code>token</code> address should be permitted. An empty <code>tokenIds</code> array should not be permitted.</li>
</ul>
<h3 id="client-response">Client response</h3>
<p>Infinex acknowledged the issue and indicated that it will be addressed in a future release.</p>
<a name="IO-IFX-RVIV-005"></a><h2 id="io-ifx-rviv-005-type-hash-does-not-conform-to-eip-712" class="break-before"><strong>IO-IFX-RVIV-005</strong> Type hash does not conform to EIP-712</h2>
<table class="metadata">

<tbody>
<tr>
<td class="rating-low">Low</td>
<td class="status-resolved">Resolved</td>
<td><a href="https://github.com/infinex-xyz/infinex-contracts/blob/d08bde067095c70c8ad03458311a118ceb7576f8/src/accounts/storage/Cosigning.sol#L12">Cosigning.sol#L12</a></td>
</tr>
</tbody>
</table>
<p>The function, <code>_createAndSignTransferRequest</code>, used to derive the struct hash for the <code>TransferRequest</code> struct, was not correctly implemented as per <a href="https://eips.ethereum.org/EIPS/eip-712">EIP-712</a>.</p>
<p>When calculating an EIP-712 type hash, non-fixed arrays and dynamic length elements should be tightly packed using <code>abi.encodePacked</code> and then hashed. The <code>_createAndSignTransferRequest</code> function incorrectly used <code>abi.encode</code> for the <code>amounts</code>, <code>tokenIds</code> and <code>data</code> elements.</p>
<p>Additionally, the <code>encodeType</code> definition should contain the original data types, as it provides the blueprint for generating the type hash. Without the original data types, frontend libraries will not be able to encode the data correctly.</p>
<h3 id="recommendation-1">Recommendation</h3>
<p>The <code>TRANSFER_REQUEST_TYPEHASH</code> definition should be updated to reflect the correct data types, as shown below:</p>
<pre tabindex="0" class="chroma"><code><span class="line"><span class="cl"><span class="n">TransferRequest</span><span class="p">(</span><span class="kt">bytes4</span> <span class="nb">selector</span><span class="p">,</span><span class="kt">address</span> <span class="n">destination</span><span class="p">,</span><span class="kt">address</span> <span class="n">token</span><span class="p">,</span><span class="kt">uint256</span> <span class="n">nonStandardIndex</span><span class="p">,</span><span class="kt">uint256</span> <span class="n">tokenId</span><span class="p">,</span><span class="kt">uint256</span> <span class="n">amount</span><span class="p">,</span><span class="kt">uint256</span><span class="p">[]</span> <span class="n">amounts</span><span class="p">,</span><span class="kt">uint256</span><span class="p">[]</span> <span class="n">tokenIds</span><span class="p">,</span><span class="kt">bytes</span> <span class="nb">data</span><span class="p">,</span><span class="kt">uint256</span> <span class="n">nonce</span><span class="p">)</span>
</span></span></code></pre><p>The dynamic length array elements should be encoded using <code>abi.encodedPacked</code> before hashing in <code>_createAndSignTransferRequest</code>.</p>
<h3 id="client-response-1">Client response</h3>
<p>Fixed in <a href="https://github.com/infinex-xyz/infinex-contracts/commit/14cfd332787ff017aec8e36c0f7986f1bf893269">14cfd33</a> and <a href="https://github.com/infinex-xyz/infinex-contracts/commit/f322ba3f70af339e488e4df06a80c608bb059223">f322ba3</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 width="20%"><strong>IO-IFX-PNVT-002</strong></td>
<td><a href="https://github.com/infinex-xyz/infinex-contracts/blob/42253373a0fec113f9fad879ff0fa1ece99a0892/src/accounts/utils/WithdrawUtil.sol#L153">WithdrawUtil.sol#L153</a>, <a href="https://github.com/infinex-xyz/infinex-contracts/blob/42253373a0fec113f9fad879ff0fa1ece99a0892/src/libraries/NonStandardToken.sol#L21">NonStandardToken.sol#L21</a></td>
<td>Hardcoded values are used at various points in the codebase as identifiers for non-standard NFT tokens, which can lead to a mismatch between the different lookup tables. To improve the codebase's readability and maintainability, the non-standard NFT identifiers should be defined as an <code>enum</code> in the <code>NonStandardToken</code> library.</td>
</tr>
<tr>
<td><strong>IO-IFX-PNVT-003</strong></td>
<td><a href="https://github.com/infinex-xyz/infinex-contracts/blob/42253373a0fec113f9fad879ff0fa1ece99a0892/src/accounts/utils/WithdrawUtil.sol#L153">WithdrawUtil.sol#L153</a></td>
<td>The function <code>_transferNonStandardNFT</code> reimplements the transfer logic for supported non-standard NFTs, as opposed to using the <code>transfer()</code> function already defined in the <code>NonStandardToken</code> library.</td>
</tr>
<tr>
<td><strong>IO-IFX-PNVT-004</strong></td>
<td><a href="https://github.com/infinex-xyz/infinex-contracts/blob/42253373a0fec113f9fad879ff0fa1ece99a0892/src/accounts/utils/WithdrawUtil.sol#L152">WithdrawUtil.sol#L152</a></td>
<td>The <code>_transferNonStandardNFT</code> function emits a <code>TransferNonStandardNFTExecuted</code> event even though another event with identical properties always precedes it. To avoid duplicating state, the event from <code>_transferNonStandardNFT</code> should be removed. This will result in only <code>WithdrawalNonStandardNFTExecuted</code> or <code>NonStandardNFTTransferredToApp</code> events being emitted.</td>
</tr>
</tbody>
</table>
<ol>
<li>Won't fix.</li>
<li>Fixed in commit <a href="https://github.com/infinex-xyz/infinex-contracts/commit/d43ede49be3c0c8e0ad7683391e697679e2d19ba">d43ede4</a>.</li>
<li>Fixed in commit <a href="https://github.com/infinex-xyz/infinex-contracts/commit/d43ede49be3c0c8e0ad7683391e697679e2d19ba">d43ede4</a>.</li>
</ol>
<h1 id="specification">Specification</h1>
<p>The following section outlines the system's intended functionality at a high level, as derived from the codebase implementation and the specification in <a href="https://proposals.infinex.xyz/xips/xip-91">XIP-91</a>. Any conflicts or ambiguities should be flagged with the auditing team for resolution.</p>
<h3 id="transfers">Transfers</h3>
<p>A key enhancement to Infinex Accounts is the introduction of functionality to transfer assets to external addresses outside the Infinex ecosystem.</p>
<p>To mitigate risks such as phishing scams and other common Web3 attacks, all transfer requests require cosignature by Infinex. This cosignature process is expected to include multi-factor authentication (e.g., TOTP) to ensure the secure use of the Platform Key.</p>
<h3 id="recovery-address-migration">Recovery address migration</h3>
<p>Users now have the ability to update their nominated recovery address, a feature that was previously restricted to a one-time setup, typically during account creation. To prevent misuse, any change to the recovery address must be cosigned by Infinex. It is anticipated that Infinex will enforce multi-factor authentication or employ a risk-based strategy to validate such requests.</p>
<h3 id="compatibility-enhancements">Compatibility enhancements</h3>
<p>Several compatibility updates have been introduced, including:</p>
<ul>
<li><em>ERC721A Support:</em> Enabling batch transfers for ERC721A tokens, such as Infinex's Patron NFT.</li>
<li><em>Non-Standard NFTs:</em> Adding transfer, recovery, and withdrawal support for non-standard NFTs, including CryptoPunks and EtherRocks.</li>
<li><em>EIP-1271 Compliance:</em> Introducing a public function to validate whether the sudo key has signed a given message hash.</li>
</ul>

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

‍