- Hacken
- Audits
- grace-protocol
- [SCA] [Re-Audit] Grace Protocol | Lending Platform | Apr2024
Audit name:
[SCA] [Re-Audit] Grace Protocol | Lending Platform | Apr2024
Date:
May 14, 2024
Table of Content
Introduction
We express our gratitude to the Grace Protocol team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.
Grace is an L2-first cross-margin lending protocol that fairly distributes losses if they occur, making it resilient to oracle manipulation, volatility and exploits.
| title | content |
|---|---|
| Platform | EVM |
| Language | Solidity |
| Tags | Lending, ERC20, Oracle |
| Timeline | 12/04/2024 - 25/04/2024 |
| Methodology | https://hackenio.cc/sc_methodology→ |
Review Scope | |
|---|---|
| Repository | https://github.com/nourharidy/grace-protocol→ |
| Commit | bb62016 |
Review Scope
- Commit
- bb62016
Audit Summary
10/10
63%
9/10
6/10
The system users should acknowledge all the risks summed up in the risks section of the report
Document Information
This report may contain confidential information about IT systems and the intellectual property of the Customer, as well as information about potential vulnerabilities and methods of their exploitation.
The report can be disclosed publicly after prior consent by another Party. Any subsequent publication of this report shall be without mandatory consent.
Document | |
|---|---|
| Name | Smart Contract Code Review and Security Analysis Report for Grace Protocol |
| Audited By | David Camps Novi, Viktor Lavrenenko |
| Approved By | Przemyslaw Swiatowiec |
| Website | https://app.grace.loans→ |
| Changelog | 29/04/2024 - Preliminary Report ; 16/05/24 - Final Report |
Document
- Name
- Smart Contract Code Review and Security Analysis Report for Grace Protocol
- Audited By
- David Camps Novi, Viktor Lavrenenko
- Approved By
- Przemyslaw Swiatowiec
- Website
- https://app.grace.loans→
- Changelog
- 29/04/2024 - Preliminary Report ; 16/05/24 - Final Report
System Overview
Grace is an L2-first cross-margin lending protocol that fairly distributes losses if they occur, with the following contracts:
BorrowController — Handles borrowing related logic and calculations.
Pool —Pools are deployed for each borrowable token via the core contract only. Pool contracts are the user-facing entry-points for users looking for debt-related operations such as lending and borrowing.
Collateral — Collaterals are deployed for each token used as collateral via the core contract only. Collateral contracts are the user-facing entry-points for users looking for collateral-related operations such as depositing or withdrawing collateral.
Oracle — The Oracle contract provides the core with prices of all collateral and pool tokens.
RateProvider — The rate provider manages the interest rate model contracts for each pool and fee rate model for each collateral.
Vault — Vaults allow Pool share token holders to deposit their shares into the vault in order to earn GTR rewards.
VaultFactory — The factory is used by the owner to deploy new vaults. It is also used by the owner in order to assign different reward weights for each vault.
Reserve —The reserve acts as the feeRecipient of the core contract. It receives all interest and collateral fees generated by the protocol.
Core — The core contract is a monolithic contract that contains hook functions.
GTR — The token used to pay for interests.
PoolDeployer — Deploys pool contracts.
CollateralDeployer — Deploys collateral contracts.
Privileged roles
The
BorrowControllercontract has the following roles:The owner, which can:
set a new owner
set a guardian
forbid the contracts
choose the allowed contracts
suspend the borrowing for a specific pool
set daily borrow limit in USD
The guardian, which can:
pause the borrowing for a specific pool
The
Corecontract has the following roles:The owner, which can:
set a new owner
set the address of the
Oracleset the address of the
BorrowControllerset the address of the
RateProviderset the address of the
PoolDeployerset the address of the
CollateralDeployerset the address of the fee receiver
set liquidation incentive in bps
set the maximum liquidation incentive in USD
set the BadDebtCollateralThresholdUsd
set the WriteOff incentive Bps
deploy a new pool
set a pool deposit cap
deploy a new collateral
set a collateral factor
set a collateral cap in USD
pull tokens from the
Corecontractpull tokens from the
Poolcontractpull tokens from the
Collateralcontract
The
GTRcontract has the following roles:The operator, which can:
set minters
set a new operator
The
Oraclecontract has the following roles:The owner, which can:
set a new owner
set an address of the collateral feed
set an address of the pool feed
set the pool fixed price
set the bps per week
The
Poolcontract has the following roles:The core, which can pull ERC20 tokens from the pool
The
RateProvidercontract has the following roles:The owner, which can:
set a new owner
set a default interest rate model
set a default collateral fee model
set an interest rate model
set a collateral fee model
The
Reservecontract has the following roles:The owner, which can:
set a new owner
request and execute allowance
The
VaultFactorycontract has the following roles:The operator, which can:
create new vaults
set weights
set a new operator
set a reward budget
Executive Summary
Documentation quality
The total Documentation Quality score is 6 out of 10.
Functional requirements are limited.
Missing roles description.
Functional requirements are superficial.
The technical description is not complete.
Missing NatSpec.
Technical specifications are not provided.
Code quality
The total Code Quality score is 9 out of 10.
Some best practices are followed: F-2024-1753, F-2024-1745, F-2024-1762.
The development environment is configured.
Test coverage
Code coverage of the project is 63% (branch coverage).
Deployment and basic user interactions are not covered with tests.
Negative cases coverage is missed.
Interactions by several users are not tested thoroughly.
Security score
Upon auditing, the code was found to contain 0 critical, 0 high, 2 medium, and 2 low severity issues, leading to a security score of 10 out of 10.
All identified issues are detailed in the “Findings” section of this report.
Summary
The comprehensive audit of the customer's smart contract yields an overall score of 8. This score reflects the combined evaluation of documentation, code quality, test coverage, and security aspects of the project.
Risks
Iterating over a dynamic array populated with custom length can lead to gas limit denial of service if the number of elements goes out of control. This scenario is possible in such as Core::writeOff, where several loops take place.
The ERC20 contract contains extra vesting logic. This may cause it to be non applicable to generic ERC20 accepting procedures.
The potential for future account abstraction mechanisms in Ethereum could alter the behavior of tx.origin, possibly bypassing this check.
Despite conditional usage, a malicious contract could still exploit tx.origin under certain circumstances, such as through carefully crafted call chains.
The system benefits different uint types, conversions and calculations between these different types may lead to unexpected results due to limitations.
Users should be aware that the fee the system charges can change over time.
The GTR does not have a maximum supply, only limited to 2**96. Given this limit is reached, functions that require a token minting will revert.
The system allows any token to be used. However, the development team stated that supported tokens will be pre-approved to ensure that they're compliant with ERC20, do not introduce any reentrancy risk and are non-rebasing.
The project utilizes Solidity version 0.8.20 or higher, which includes the introduction of the PUSHO (0x5f) opcode. This opcode is currently supported on the Ethereum mainnet but may not be universally supported across other blockchain networks. Consequently, deploying the contract on chains other than the Ethereum mainnet, such as certain Layer 2 (L2) chains or alternative networks, might lead to compatibility issues or execution errors due to the lack of support for the PUSHO opcode. In scenarios where deployment on various chains is anticipated, selecting an appropriate Ethereum Virtual Machine (EVM) version that is widely supported across these networks is crucial to avoid potential operational disruptions or deployment failures.
The function Reserve::requestAllowance allows to override the allowanceRequest, which may result in undesired behavior.
The protocol interacts with external, non-trusted, out-of-scope contracts.
The project's contracts don't implement the emergency stop pattern, which might lead to problems in the future.
The Vault contract sets an unlimited allowance for the Pool to use its asset tokens. It creates risks and can lead to unexpected behavior.
The owner of the Core contract can pull stuck ERC20 tokens from the Core, Pool and Collateral contracts, which creates a risk of highly permissive role access. It is recommended to use a MultiSignature wallet.
In the case that an oracle stops providing reliable token prices, the system will be at risk for a particular amount of time, until a new oracle is setup. The team implemented several measures to mitigate the scenario, explained in the finding F-2024-1736 that should be reviewed.
Zero address validation is handled off-chain. However, it can lead to problems during the direct interaction with the protocol.
Findings
Code ― | Title | Status | Severity | |
|---|---|---|---|---|
| F-2024-1736 | Chainlink’s latestRoundData might return stale or incorrect results | accepted | Medium | |
| F-2024-1629 | The usage of the precompile ecrecover can lead to signature mailability | fixed | Medium | |
| F-2024-1787 | Privileged roles should follow two step ownership transfer pattern | accepted | Low | |
| F-2024-1747 | Use of transfer() instead of call() to send native tokens | fixed | Low | |
| F-2024-1784 | Missing feed address check may result in undesired price calculations | fixed | Observation | |
| F-2024-1779 | Lack of consistency in assets redeemed amount | fixed | Observation | |
| F-2024-1762 | Missing events emitting for critical data | accepted | Observation | |
| F-2024-1755 | Cache array length in loops to save gas | fixed | Observation | |
| F-2024-1753 | The EIP712 domain separator is missing the version field | accepted | Observation | |
| F-2024-1751 | Missing call to moveDelegates in GTR burn | fixed | Observation |
Appendix 1. Severity Definitions
When auditing smart contracts, Hacken is using a risk-based approach that considers Likelihood, Impact, Exploitability and Complexity metrics to evaluate findings and score severities.
Reference on how risk scoring is done is available through the repository in our Github organization:
Severity | Description |
|---|---|
Critical | Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation. |
High | High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation. |
Medium | Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category. |
Low | Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution, do not affect security score but can affect code quality score. |
Severity
- Critical
Description
- Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation.
Severity
- High
Description
- High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation.
Severity
- Medium
Description
- Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category.
Severity
- Low
Description
- Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution, do not affect security score but can affect code quality score.
Appendix 2. Scope
The scope of the project includes the following smart contracts from the provided repository:
Scope Details | |
|---|---|
| Repository | https://github.com/nourharidy/grace-protocol→ |
| Commit | bb62016 |
| Remediation commit | d961752 |
| Whitepaper | n/a |
| Requirements | https://docs.grace.loans→ |
| Technical Requirements | https://github.com/nourharidy/grace-protocol→ |
Scope Details
- Commit
- bb62016
- Remediation commit
- d961752
- Whitepaper
- n/a
- Requirements
- https://docs.grace.loans→
- Technical Requirements
- https://github.com/nourharidy/grace-protocol→