The audit makes no statements or warranties about utility of the code, safety of the code, suitability of the business model, investment advice, endorsement of the platform or its products, regulatory regime for the business model, or any other statements about fitness of the contracts to purpose, or their bug free status. The audit documentation is for discussion purposes only.
A group of auditors are involved in the work on the audit. The security engineers check the provided source code independently of each other in accordance with the methodology described below:
- Project documentation review.
- General code review.
- Reverse research and study of the project architecture on the source code alone.
- Build an independent view of the project's architecture.
- Identifying logical flaws.
- Manual code check for vulnerabilities listed on the Contractor's internal checklist. The Contractor's checklist is constantly updated based on the analysis of hacks, research, and audit of the clients' codes.
- Code check with the use of static analyzers (i.e Slither, Mythril, etc).
Eliminate typical vulnerabilities (e.g. reentrancy, gas limit, flash loan attacks etc.).
- Detailed study of the project documentation.
- Examination of contracts tests.
- Examination of comments in code.
- Comparison of the desired model obtained during the study with the reversed view obtained during the blind audit.
- Exploits PoC development with the use of such programs as Brownie and Hardhat.
Detect inconsistencies with the desired model.
- Cross check: each auditor reviews the reports of the others.
- Discussion of the issues found by the auditors.
- Issuance of an interim audit report.
- Double-check all the found issues to make sure they are relevant and the determined threat level is correct.
- Provide the Customer with an interim report.
- The Customer either fixes the issues or provides comments on the issues found by the auditors. Feedback from the Customer must be received on every issue/bug so that the Contractor can assign them a status (either "fixed" or "acknowledged").
- Upon completion of the bug fixing, the auditors double-check each fix and assign it a specific status, providing a proof link to the fix.
- A re-audited report is issued.
- Verify the fixed code version with all the recommendations and its statuses.
- Provide the Customer with a re-audited report.
- The Customer deploys the re-audited source code on the mainnet.
- The Contractor verifies the deployed code with the re-audited version and checks them for compliance.
- If the versions of the code match, the Contractor issues a public audit report.
- Verify the fixed code version with all the recommendations and its statuses.
- Provide the Customer with a re-audited report.
All vulnerabilities discovered during the audit are classified based on their potential severity and have the following classification:
Severity | Description |
---|---|
Critical | Bugs leading to assets theft, fund access locking, or any other loss funds to be transferred to any party. |
High | Bugs that can trigger a contract failure. Further recovery is possible only by manual modification of the contract state or replacement. |
Medium | Bugs that can break the intended contract logic or expose it to DoS attacks, but do not cause direct loss funds. |
Low | Bugs that do not have a significant immediate impact and could be easily fixed. |
Based on the feedback received from the Customer regarding the list of findings discovered by the Contractor, they are assigned the following statuses:
Status | Description |
---|---|
Fixed | Recommended fixes have been made to the project code and no longer affect its security. |
Acknowledged | The Customer is aware of the finding. Recommendations for the finding are planned to be resolved in the future. |
IronBank is a fork of Compound Protocol with some functionality improvement, such as ability to provide uncollaterized credit to external protocols, supporting of Chainlink and Band price feed and some architecture improvements.
Title | Description |
---|---|
Client | IronBank |
Project name | IronBank |
Timeline | 01-08-2022 - 05-09-2022 |
Number of Auditors | 3 |
Date | Commit Hash | Note |
---|---|---|
05.08.2019 | f385d71983ae5c5799faae9b2dfea43e5cf75262 | Audited by Trail of Bits, report |
01.08.2022 | 8cd45803b48552e344e22be280c9e1c03ec8644a | Changes from previous audit |
22.08.2022 | 5581327fc855c2734d65a5dd8f198bca6f8963e8 | Commit with fixes of findings discovered by MixBytes |
29.08.2022 | 4883f8a6d6faeafa82f7b1979cd77c8cc2b59b5c | Final commit |
The audit covered the following files:
Severity | # of Findings |
---|---|
CRITICAL | 0 |
HIGH | 0 |
MEDIUM | 5 |
LOW | 2 |
During the audit, 5 findings of medium severity were identified and confirmed by the Client. The Client have fixed 3 issues of medium severity, and 2 issues have been acknowledged. Those issues do not have a significant impact and can be resolved through careful deployment and maintenance procedures.
Not found
Not found
Fixed in https://github.com/ibdotxyz/compound-protocol/commit/66ca60479b4850fcaa60e6145fa5d671417400f1
Pausing the guardian can set a new credit limit for users with the credit limit. https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/Comptroller.sol#L1313
We recommend updating the checks in the "_setCreditLimit" function.
Acknowledged
An exchange rate bug for new pools and empty pools (without borrowers and suppliers) for CToken contracts without the 'internalCash' variable. Flow:
- Create cToken
- Mint cToken by user1 (1,000,000)
- Redeem cToken by user1 (999,999.999999)
- Transfer underlying (1,000,000) from user1 to market
- Mint cToken by user2 (1,000,000)
- Redeem cToken by user1 (user1 receive extra tokens)
We recommend checking the exchange rate before the first mint or using the 'internalCash' value for all CToken contracts.
No need to fix. All new pools will be deployed with CCollateralCap implementation which has internalCash to prevent such vulnerability.
Fixed in https://github.com/CreamFi/compound-protocol/commit/2773bdaa91c3782561337aabd5dd8cc3730b49d6
After an admin changes the interest rate model parameters by using this function https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/TripleSlopeRateModel.sol#L100 indexes will be recalculated in the upcoming accrueInterest() function call. But this call applies new interest settings to the previous period of time which is not correct.
The interest rate model parameters should be changed just after calling the accrueInterest() function for each asset. It can be done by creating a special service contract.
Acknowledged
Let's take a look at the flashloan flow. After doTransferOut a receiver gets amount - fee
.
https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20.sol#L217
Then a receiver's onFlashLoan
function will be called with an incorrect amount.
https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20.sol#L224
Then doTransferIn will transfer the repayment amount but the contract will receive the repayment amount - fee
https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20.sol#L231 and therequire
check will cause a revert.
https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20.sol#L235
The flashloan() function should be rewritten taking into consideration the USDT fee value.
No need to fix. Currently there is no USDT fee value. Since it affects Flash Loan only, we consider it acceptable to fix the issue reactively along with Compound.
Fixed in https://github.com/ibdotxyz/compound-protocol/commit/66ca60479b4850fcaa60e6145fa5d671417400f1
In the IronBank, the credit
functionality is introduced. An admin can trigger the setCreditLimit
function to mark some addresses like credit
for specific cToken
and set its credit limit
. Such addresses can borrow a limited by credit limit
amount of cToken
without providing any collateral. Additionally, setCreditLimit
can mark that an address is no longer credit
and has become an ordinary account that requires collateral.
Unfortunately, after becoming an ordinary account, the ex-credit account will be subject to repay and/or liquidation of its borrowed debt.
Although an attack is hard to implement since the setCreditLimit
function is restricted to the admin, we recommend to disallow changing the state from the credit account to an ordinary account, e.g. by disallowing setting the credit limit less than the currently borrowed amount.
Fixed in https://github.com/CreamFi/compound-protocol/commit/a059518d058fcfd34497c486bf5acb92b31c550e
Several typos:
- 'occured' instead of 'occurred' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CTokenInterfaces.sol#L366 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CTokenInterfaces.sol#L412
- 'fucntions' instead of 'functions' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/PriceOracle/PriceOracleProxy.sol#L160 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/PriceOracle/PriceOracleProxyIB.sol#L96
- 'supplys' instead of 'supplies' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/Comptroller.sol#L1144
- 'currenlty' instead of 'currently' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/PriceOracle/PriceOracleProxy.sol#L189 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/PriceOracle/PriceOracleProxyIB.sol#L101
- 'settor' instead of 'setter' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CErc20Delegator.sol#L54
- 'accuring' instead of 'accruing' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20.sol#L556 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20.sol#L595 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20CheckRepay.sol#L557 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20CheckRepay.sol#L596 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20NoInterest.sol#L557 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CCollateralCapErc20NoInterest.sol#L596 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CErc20.sol#L300 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CErc20.sol#L405 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CErc20.sol#L475 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CWrappedNative.sol#L659 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CWrappedNative.sol#L729
- 'sieze' instead of 'seize' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/Comptroller.sol#L595
- 'depreacted' instead of 'deprecated' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L100 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L104 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L108 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L112 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L116 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L120 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/ComptrollerStorage.sol#L136
- 'undelrying' instead of 'underlying' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CToken.sol#L540 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CTokenCheckRepay.sol#L540 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CTokenNoInterest.sol#L569
- 'tather' instead of 'rather' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CToken.sol#L1040 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/CTokenCheckRepay.sol#L1059
- 'amnount' instead of 'amount' https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/InterestRateModel.sol#L15 https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/InterestRateModel.sol#L28
We recommend correcting them.
Fixed in https://github.com/CreamFi/compound-protocol/commit/f90921f07627a60707fc09bec6acc43cb2999f0e
Some code lacks a check for null address: admin: https://github.com/ibdotxyz/compound-protocol/blob/8cd45803b48552e344e22be280c9e1c03ec8644a/contracts/PriceOracle/PriceOracleProxyIB.sol#L157
We recommend adding null checks.
MixBytes is a team of blockchain developers, auditors and analysts keen on decentralized systems. We build opensource solutions, smart contracts and blockchain protocols, perform security audits, work on benchmarking and software testing solutions, do research and tech consultancy.