{"id":620,"date":"2023-12-18T17:26:54","date_gmt":"2023-12-18T15:26:54","guid":{"rendered":"https:\/\/ackeeblockchain.com\/blog\/?p=620"},"modified":"2024-07-04T12:54:57","modified_gmt":"2024-07-04T10:54:57","slug":"ipor-protocol-core-audit-summary","status":"publish","type":"post","link":"https:\/\/ackee.xyz\/blog\/ipor-protocol-core-audit-summary\/","title":{"rendered":"IPOR: Protocol Core audit summary"},"content":{"rendered":"<p><a href=\"https:\/\/www.ipor.io\/\" target=\"_blank\" rel=\"noopener\"><span style=\"font-weight: 400;\">IPOR protocol<\/span><\/a><span style=\"font-weight: 400;\"> (Inter Protocol Over-block Rate) consists of two parts: the IPOR index and the automated market maker.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">The index functions as an interest rate benchmark, aggregating the lending and borrowing interest rates from multiple DeFi lending platforms to provide an objective view of the current market status. The logic lies off-chain. To access the index, a user must call the IPOR Oracle contract.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">The second part is the IPOR automated market maker. Unlike the standard AMM, the IPOR AMM allows users to open\/close swaps and speculate on interest rates. Swaps can be opened in two different directions:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">To pay a fixed interest rate and receive a floating interest rate<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">To pay a floating interest rate and receive a fixed interest rate<\/span><\/li>\n<\/ul>\n<p><span style=\"font-weight: 400;\">The vital role is a liquidity provider. Liquidity providers provide liquidity to the AMM and receive a share of the swap fees. Liquidity in the pool is automatically rebalanced between the protocol treasury and the asset management, which deposits assets into Aave or Compound to earn interest.<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Revision 1.0 &amp; 1.1<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">IPOR engaged Ackee Blockchain to perform a security review of the IPOR protocol with a total time donation of 59 engineering days in a period between June 1 and July 28, 2023. Revisions 1.0 and 1.1 were performed during one review. <\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Revision 1.2\u00a0<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">IPOR provided an updated codebase with fixes for several reported issues on the 22nd of August, 2023. However, not all the issues were fixed. Together with fixes, <\/span><span style=\"font-weight: 400;\">new contracts <\/span><span style=\"font-weight: 400;\">for staked ETH pool were introduced. <\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Revision 1.3 &amp; 1.4<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">IPOR provided an updated codebase with fixes and a few code changes on the date 18th of September, 2023, the review was done with a time donation of 3 MD. For Revision 1.4 IPOR provided an updated codebase with fixes on the commit <code class=\"codehl\">3d99b22<\/code>. No new changes were introduced except for updating the values of the spread slope.<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Revision 2.0<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">IPOR reached out to Ackee Blockchain to perform an incremental security review of an updated version of the codebase<\/span> <span style=\"font-weight: 400;\">\u00a0<\/span><span style=\"font-weight: 400;\">with a total time donation of 7 engineering days in a period between December 4 and December 11, 2023.\u00a0<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Revision 2.1\u00a0<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">IPOR provided an updated codebase with fixes for the reported issues. The codebase also contains changes to the stETH demand spread model and additional code refactoring.\u00a0<\/span><\/p>\n<p>&nbsp;<\/p>\n<h2>Methodology<\/h2>\n<p><strong>Revision 1.0 &amp; 1.1<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">We began our review using static analysis tools <\/span><a title=\"Wake: New open-source tooling on Ethereum to stop bugs\" href=\"https:\/\/ackeeblockchain.com\/blog\/wake-new-open-source-tooling-on-ethereum-to-stop-bugs\/\" target=\"_blank\" rel=\"noopener\"><span style=\"font-weight: 400;\">Wake<\/span><\/a><span style=\"font-weight: 400;\">. <\/span><span style=\"font-weight: 400;\">We then took a deep dive into the logic of the contracts and performed a manual code review. In parallel with the manual review, we created comprehensive fuzz tests for the most critical parts of the system, such as opening\/closing swaps and providing liquidity. For testing and fuzzing, we have involved <\/span><span style=\"font-weight: 400;\">Wake <\/span><span style=\"font-weight: 400;\">testing framework. The test simulates the real-world behavior with a complete deployment of the protocol and with a focus on unexpected call sequences and edge case values. <\/span><\/p>\n<p><span style=\"font-weight: 400;\">During the fuzz testing, we found several issues that were reported to the client and immediately fixed. The cooperation with the team and their ability to quickly react was crucial to fuzz testing, where a correctly working protocol is necessary as it allows us to find more issues and edge cases. Fuzz testing at its final stage ran on newer commits (with bug fixes) than the initial one. This commit will be mentioned in Revision 1.1.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">During the review, we paid particular attention to:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ensuring the arithmetic of the system is correct<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">detecting possible financial attacks such as flash loans<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ensuring the protocol\u2019s behavior stays consistent in edge case scenarios<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking access control mechanisms<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">detecting possible reentrancies in the code<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking proper storage handling<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">comparing the code logic to the documentation<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">looking for common issues such as data validation.\u00a0<\/span><\/li>\n<\/ul>\n<p>&nbsp;<\/p>\n<p><strong>Revision 1.2<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">In the first part of the review, we reviewed all the fixes and ensured they corresponded with our recommendations.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">In the second part of the review, we manually reviewed the new codebase containing four new contracts and changes in several older contracts. The new codebase allows providing liquidity with Ether, Wrapped Ether, and Staked Ether.<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><span style=\"font-weight: 400;\"><strong>Revision 1.3<\/strong> <\/span><\/p>\n<p><span style=\"font-weight: 400;\">We began the review with a focus on fixes of previously discovered issues. Then, we focused on the new changes in the codebase. After the manual review, we updated the fuzz tests to be relevant to the new codebase.<\/span><span style=\"font-weight: 400;\"><br \/>\n<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Revision 2.0<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">Our review began with updating the fuzz test to be relevant to the new refactored codebase.\u00a0<\/span><\/p>\n<p><span style=\"font-weight: 400;\">After the original fuzz test was updated, we started writing a new fuzz test for the latest part of the protocol managing the Staked ETH. With the new fuzz test, we discovered the two most severe issues reported in this revision.\u00a0<\/span><\/p>\n<p><span style=\"font-weight: 400;\">In parallel with the fuzz testing, we performed a manual review, ran <\/span><span style=\"font-weight: 400;\">Wake <\/span><span style=\"font-weight: 400;\">static analysis, and discussed all detections.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">During the review we were focusing <\/span><span style=\"font-weight: 400;\">on the following code changes:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">new risk indicators approach with the parameters being signed and passed directly to the contracts<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">new stETH feature \u2014 opening and closing swaps<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">a different model of spread logic for stETH<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">admin-only setter for time-weighted notional data<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">the no-closing period after opening a swap<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">code refactoring and new architecture compatibility.\u00a0<\/span><\/li>\n<\/ul>\n<p>&nbsp;<\/p>\n<h2><b>Scope\u00a0<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">Revision 1.0: <\/span><span style=\"font-weight: 400;\">The audit was initially performed on the commit <code class=\"codehl\">680c80f<\/code>.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">Revision 1.1: The review then continued on the last provided commit <code class=\"codehl\">87c4d345<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">Revision 1.2: The review was performed on the commit <code class=\"codehl\">1847f3e6<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">Revision 1.3: The review was performed on the commit <code class=\"codehl\">553e1c7<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">Revision 1.4: <\/span><span style=\"font-weight: 400;\">The review was performed on an updated codebase with fixes on the commit <code class=\"codehl\">3d99b22<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">Revision 2.0: The audit was performed on the commit <code class=\"codehl\">2c633063<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">Revision 2.1: The audit was performed on an updated codebase with fixes for the reported issues on the commit <code class=\"codehl\">125b3f3<\/code><\/span><\/p>\n<p>&nbsp;<\/p>\n<h2><b>Findings<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">Here we present our <\/span><span style=\"font-weight: 400;\">findings<\/span><span style=\"font-weight: 400;\">.<\/span><\/p>\n<p>&nbsp;<\/p>\n<h3><b>Critical severity<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">C1: Profit &amp; loss accounted twice when unwinding<\/span><\/p>\n<p>&nbsp;<\/p>\n<h3><b>High severity\u00a0<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">H1: Unwinding formula<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H2: Broken reentrancy lock<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H3: Unwinding fee accounted twice in <\/span><span style=\"font-weight: 400;\">liquidityPool <\/span><span style=\"font-weight: 400;\">balance\u00a0<\/span><\/p>\n<p>&nbsp;<\/p>\n<h3><b>Medium severity<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">M1: <code class=\"codehl\">INTEREST_FROM_STRATEGY_BELOW_ZERO<\/code> reverts\u00a0<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M2: Inaccurate hypothetical interest formula<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M3: Pool contribution is not updated when liquidity is redeemed<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M4: Incorrect event data<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M5: Unwinding fee normalization<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M6: <code class=\"codehl\">IPOR_508<\/code> reverts during deposit<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M7: Liquidation deposits accounted into LP balance <\/span><\/p>\n<p>&nbsp;<\/p>\n<h3><b>Low severity<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">L1: Value in incorrect decimals<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L2: Liquidation deposit accounted twice in rebalancing logic<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L3: Aave incorrect APY formula<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L4: Close swap and redeem transaction reverts<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L5: No data validation while setting <code class=\"codehl\">redeemFeeRateEth<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">L6: Close swap insufficient balance revert<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L7: <code class=\"codehl\">IporProtocolRouter<\/code> return &amp; revert data dropped\u00a0<\/span><\/p>\n<p>&nbsp;<\/p>\n<h3><b>Warning severity\u00a0<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">W1: Usage of <code class=\"codehl\">solc<\/code> optimizer<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W2: <code class=\"codehl\">SoapIndicatorRebalanceLogic<\/code> underflow<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W3: Insufficient data validation in the constructor<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W4: Missing array length check in the initialize function<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W5: <code class=\"codehl\">_calculateRedeemedCollateralRatio<\/code> underflow<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W6: Constant block production relied on<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W7: Github secrets leak<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W8: Infinite approval<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W9: Missing swap direction validation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W10: Setting array max index in constructor<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W11: <\/span><span style=\"font-weight: 400;\">IporProtocolRouter <\/span><span style=\"font-weight: 400;\">memory constraints violation\u00a0<\/span><\/p>\n<p>&nbsp;<\/p>\n<h3><b>Informational severity <\/b><\/h3>\n<p>I1: Unreachable code<\/p>\n<p>I2: Use <code class=\"codehl\">type(uint256).max<\/code> instead of integer literal<\/p>\n<p>I3: Duplicated code<\/p>\n<p>I4: Redundant require<\/p>\n<p>I5: Using magic numbers<\/p>\n<p>I6: Use <code class=\"codehl\">forceApprove<\/code> instead of <code class=\"codehl\">safeApprove<\/code><\/p>\n<p>I7: User can lose funds if the protocol is used incorrectly<\/p>\n<p>I8: Mixing <code class=\"codehl\">_msgSender()<\/code> and <code class=\"codehl\">msg.sender<\/code> across the codebase<\/p>\n<p>I9: Redundant logging of <code class=\"codehl\">block.timestamp<\/code><\/p>\n<p>I10: Unused code<\/p>\n<p>&nbsp;<\/p>\n<h2><b>CONCLUSION<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">Our review resulted in <\/span><b>39 findings across Revision 1.0 &#8211; 2.0<\/b><span style=\"font-weight: 400;\">, ranging from <\/span><i><span style=\"font-weight: 400;\">Info<\/span><\/i><span style=\"font-weight: 400;\"> to <\/span><i><span style=\"font-weight: 400;\">Critical<\/span><\/i><span style=\"font-weight: 400;\"> severity.\u00a0<\/span><\/p>\n<p><span style=\"font-weight: 400;\">As of Revision 2.0, <\/span><span style=\"font-weight: 400;\">the new codebase is more readable than the previous ones. However, there are still some parts that could be improved. The NatSpec documentation is part of interface contracts, and it does not cover all the functions and used libraries (<\/span><span style=\"font-weight: 400;\">RiskIndicatorsValidatorLib<\/span><span style=\"font-weight: 400;\">, for example). The code contains <\/span><span style=\"font-weight: 400;\">unused functions<\/span><span style=\"font-weight: 400;\">, and tests are not written based on the expected behavior but instead based on the known outputs of the code being tested.<\/span><\/p>\n<p><b>We recommend IPOR to:<\/b><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">avoid writing tests based on the outputs of the code being tested<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">use static analysis tools (such as <\/span><span style=\"font-weight: 400;\">Wake<\/span><span style=\"font-weight: 400;\">) to keep the codebase clean<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">update the documentation to reflect the current state of the protocol (concerning the new stETH part, especially).<\/span><\/li>\n<\/ul>\n<p><b>Ackee Blockchain\u2019s full <\/b><b><i>IPOR <\/i><\/b><b>audit report with a more detailed description of all findings and recommendations can be found <\/b><a href=\"https:\/\/docs.ipor.io\/audits\" target=\"_blank\" rel=\"noopener\"><b>here<\/b><\/a><b>.<\/b><\/p>\n<p><span style=\"font-weight: 400;\">We were delighted to audit<\/span><b> IPOR<\/b><span style=\"font-weight: 400;\"> and look forward to working with them again.<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><strong>Final note<\/strong><\/p>\n<p><span style=\"font-weight: 400;\">In the given time donation and after all reported issues were fixed, the auditing team doesn\u2019t see any issue that would lead to a loss of funds or any other catastrophic consequences. The confidence of the auditing team is based on a manual review and a fuzz testing model.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">The IPOR team sticks with good practices. The code quality is high, the code contains NatSpec documentation, and the general documentation is comprehensive. IPOR team also provided many diagrams and mathematical equations, which made the audit process more effective.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">We cannot rule out the chance of DoS of the protocol caused by some edge case conditions. However, it is not directly related to security but rather to the mathematical and architectural complexity of the protocol.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">The next step to enhance confidence in the protocol is to extend the fuzz test and model all the parts of the protocol that are not included in the current fuzz test (liquidity mining, governance).\u00a0<\/span><\/p>\n","protected":false},"excerpt":{"rendered":"<p>IPOR protocol (Inter Protocol Over-block Rate) consists of two parts: the IPOR index and the automated market maker. The index functions as an interest rate benchmark, aggregating the lending and borrowing interest rates from multiple DeFi lending platforms to provide an objective view of the current market status. The logic lies off-chain. To access the index, a user must call the IPOR&hellip;<\/p>\n","protected":false},"author":15,"featured_media":621,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[20,10,80,103],"tags":[21,32,24,110,68,104],"class_list":["post-620","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-audits","category-ethereum","category-solidity","category-wake","tag-audit","tag-defi","tag-ethereum","tag-ipor","tag-solidity","tag-wake"],"aioseo_notices":[],"featured_image_src":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2023\/12\/IPOR_BLOG-600x400.png","featured_image_src_square":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2023\/12\/IPOR_BLOG-600x600.png","author_info":{"display_name":"Aleksandra Yudina","author_link":"https:\/\/ackee.xyz\/blog\/author\/aleksandra-yudina\/"},"_links":{"self":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/620","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/users\/15"}],"replies":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/comments?post=620"}],"version-history":[{"count":0,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/620\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media\/621"}],"wp:attachment":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media?parent=620"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/categories?post=620"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/tags?post=620"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}