iosiro was commissioned by Set Protocol to conduct a smart contract audit on the AaveLeverageModule and DebtIssuanceModuleV2 contracts. The audit was split in two phases, the first focused on the AaveLeverageModule and was performed between 11 August and 19 August 2021. The second phase was performed between 30 August and 3 September 2021, which included the DebtIssuanceModuleV2 contract and a review of the issues reported. In total, 13 resource days were spent by a single auditor.
This report is organized into the following sections.
- Section 2 - Executive summary: A high-level description of the findings of the audit.
- Section 3 - Audit details: A description of the scope and methodology of the audit.
- Section 4 - Design specification: An outline of the intended functionality of the smart contracts.
- Section 5 - Detailed findings: Detailed descriptions of the findings of the audit.
The information in this report should be used to better understand the risk exposure of the smart contracts, and as a guide to improving the security posture of the smart contracts by remediating issues identified. The results of this audit are only a reflection of the source code reviewed at the time of the audit and of the source code that was determined to be in-scope.
The purpose of this audit was to achieve the following:
- Identify potential security flaws.
- Ensure that the smart contracts functioned according to the documentation provided.
Assessing the off-chain functionality associated with the contracts, for example, backend web application code, was out of scope of this audit.
Due to the unregulated nature and ease of transfer of cryptocurrencies, operations that store or interact with these assets are considered very high risk with regards to cyber attacks. As such, the highest level of security should be observed when interacting with these assets. This requires a forward-thinking approach, which takes into account the new and experimental nature of blockchain technologies. Strategies that should be used to encourage secure code development include:
- Security should be integrated into the development lifecycle and the level of perceived security should not be limited to a single code audit.
- Defensive programming should be employed to account for unforeseen circumstances.
- Current best practices should be followed where possible.
This report presents the findings of an audit performed by iosiro on the Set Protocol AaveLeverageModule and DebtIssuanceModuleV2 smart contracts. The contracts allow for the creation of SetTokens that leverage their positions by integrating with AaveV2. The DebtIssuanceModuleV2 was designed to support a wide variety of rebasing tokens which were incompatible with the original DebtIssuanceModule, including Aave's interesting bearing aTokens.
iosiro noted two high risk, one medium risk, and three informational findings, one of which was related to minor code quality improvements.
One of the high-risk issues related to default positions not accounted for when reducing the leveraged position, resulting in inaccessible funds, and the other related to borrow or repay hooks triggering twice when twice when issuing or redeeming via the DebtIssuanceModule.
The medium risk issue pertained to the lack of integration tests covering interactions between the module, the DebtIssuanceModuleV2 and other aspects of the Set Protocol ecosystem.
By the conclusion of the final review, all issues had been addressed or remediated. Overall, the code was found to be of a high standard and followed best practices.
The source code considered in-scope for the assessment is described below. Code from all other files was considered to be out-of-scope. Out-of-scope code that interacts with in-scope code was assumed to function as intended and not introduce any functional or security vulnerabilities.
- Project name: Set Protocol V2
- Commits:
99a57dd - Files: AaveLeverageModule.sol, AaveV2.sol, DebtIssuanceModuleV2.sol, IssuanceValidationUtils.sol
A variety of techniques, described below, were used to conduct the audit.
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.
The contracts were compiled, deployed, and tested in a Hardhat test environment, both manually and through the Hardhat test suite provided. Manual analysis was used to confirm that the code was functional and to discover whether any potential security issues identified could be exploited. The coverage report of the provided Hardhat tests as on the final day of the audit is given below.
| File | % Stmts | % Branch | % Funcs | % Lines |
|---|---|---|---|---|
| protocol/modules/DebtIssuanceModuleV2.sol | 100 | 100 | 100 | 100 |
| protocol/modules/AaveLeverageModule.sol | 100 | 100 | 100 | 100 |
| protocol/lib/IssuanceValidationUtils.sol | 100 | 100 | 100 | 100 |
| protocol/integration/lib/AaveV2.sol | 100 | 100 | 100 | 100 |
The code coverage of all new code developed for the AaveLeverageModule and DebtIssuanceModuleV2 contracts was excellent.
Tools were used to automatically detect the presence of several types of security vulnerabilities, including reentrancy, timestamp dependency bugs, and transaction-ordering dependency bugs. The static analysis results were manually reviewed and any false positives were removed from the results. Any true positive results were included in this report.
Static analysis tools commonly used include Slither, Securify, and MythX. Tools such as the Remix IDE, compilation output, and linters could also be used to identify potential areas of concern.
Each issue identified during the audit has been assigned a risk rating. The rating is determined based on the criteria outlined below.
- High risk - The issue could result in a loss of funds for the contract owner or system users.
- Medium risk - The issue resulted in the code specification being implemented incorrectly.
- Low risk - A best practice or design issue that could affect the security of the contract.
- Informational - A lapse in best practice or a suboptimal design pattern that has a minimal risk of affecting the security of the contract.
- Closed - The issue was identified during the audit and has since been addressed to a satisfactory level to remove the risk that it posed.
The following section outlines the intended functionality of the system at a high level.
The specification of STIP-001 was based on commit b9a5af8 and the implementation in the codebase. Any perceived points of conflict should be highlighted with the auditing team to determine the source of the discrepancy.
The AaveLeverageModule is a module to support leveraging of positions using AaveV2. Managers can enable this module on SetTokens that have default positions of supported Aave collateral tokens (aTokens). The module is designed to be used in conjunction with the DebtIssuanceModuleV2 for issuing and redeeming SetTokens.
The leverage process involves borrowing against collateral assets, exchanging the borrowed assets for an underlying collateral asset and finally depositing the asset for more wrapped collateral. The manager could also delverage the position with the delever() function, which reverses the process. This involves partially redeeming some collateral, exchanging it for the borrowed asset and repaying the underlying loan.
Due the order of operations, the manager could be required to delever() a SetToken in increments. If the position is near to the liquidation threshold, only a limited amount of collateral can be withdrawn. This is to ensure the loan does not become under collateralized. As a proportion of debt is repaid, more collateral can be withdrawn on subsequent calls, until enough can be withdrawn to delever() to the desired amount.
The AaveLeverageModule can make use of any of Set Protocol's approved exchange adapters to exchange the assets underlying the SetToken. The manager is responsible for providing fair amounts for the trade parameters that minimize the risk of front-running and the impact of slippage.
If a SetToken becomes under collateralized, it is possible to liquidate its position. This will effectively reduce the leverage ratio, but will incur significant penalities. It is expected that the manager will be autonomous or act reliably and ensure that a SetToken does not become subject to liquidations.
If the health factor falls below liquidationBonus x liquidationThreshold, the loan and collateral can be liquidated to zero by calling liquidationCall() repeatedly. With WETH as the only collateral asset, this health factor threshold is 1.05 x 0.825 = 0.86625, based on the current risk parameters. This is unlikely to occur, as it is expected that keeper bots will liquidate the position as soon as possible; however, it remains imperative that the manager ensures that the health factor of the SetToken is adequately maintained to prevent a catastrophic loss of funds.
The external positions of the SetTokens are synchronized before issuing and redeeming due to the wrapped collateral tokens and debt aTokens rebasing at the start of every block. Any user can also call the sync() function to synchronize the external positions with the current token balances.
The module extends on the SetToken manager capabilities and provide managers the following privileged functions:
initialize(): Register the module with the SetToken and add the desired collateral and borrow assets.registerToModule(): Register the module to the configured default issuance module.addCollateralAssets(): Enables an Aave wrapped collateral asset, which will be added as a default position on the nextsync().removeCollateralAssets(): Disables an Aave wrapped collateral asset, but does not remove the collateral asset as a default position.addBorrowAssets(): Enables borrowing of a specific asset. Enables tracking of the variable debt of the asset.removeBorrowAssets(): Disables borrowing of a specific asset if the outstanding debt is zero.lever(): Increase the levered position.delever(): Reduce the levered position.deleverToZeroBalance(): Repay all outstanding debt. Requires enough unused collateral to cover the outstanding debt.
Furthermore, the owner within the Set Protocol ecosystem is able to invoke a number of additional functions:
updateAllowedSetToken(): Enable use of the module by any SetToken.updateAnySetAllowed(): Add a SetToken to the list that is allowed to register the module.
A library, called AaveV2, was developed to facilitate the interaction with AaveV2, which is deployed as a standalone library and linked during deployment of the AaveLeverageModule.
The specification of STIP-004 was based on commit b9a5af8 and the implementation in the codebase. Any perceived points of conflict should be highlighted with the auditing team to determine the source of the discrepancy.
It was determined that the original DebtIssuanceModule was incompatible with Aave wrapped tokens (aTokens and variableDebtTokens). Tokens that rebase on transfers could result in over or under collateralization when issuing or redeeming. However, due to the strict validations in the DebtIssuanceModule, both cases resulted in reverts. To allow for slight over collateralization, the issue and redeem functions of the DebtIssuanceModule were overridden in the new DebtIssuanceModuleV2.
Validation was added to ensure that the post-transfer amounts are greater than or equal to the minimum required. Due to the manner in which the units required for issuing and redeeming were calculated, a small percentage of transactions would revert if amounts were similar in magnitude to the totalSupply. Fortunately, the number of rounding errors became negligible as the totalSupply increased and the issue and redeem amounts became smaller in proportion.
The following section details the findings of the audit.
No high risk issues were present at the conclusion of the review.
No medium risk issues were present at the conclusion of the review.
No low risk issues were present at the conclusion of the review.
No informational risk issues were present at the conclusion of the review.
If a manager was to delever() a token's position by more than the outstanding debt, the remaining debt assets would not be added as a default position and would remain unaccounted for.
This could occur when the manager specifies an excessive amount of _redeemQuantityUnits or if the exchange of collateral to debt assets was more favorable than anticipated or calculated.
To access the funds, a manager would need to manually add the debt asset as a default position or as a collateral asset. However, if Aave does not support the asset as collateral, the manager would need to make use of another module to reintroduce the assets. Any token issued or redeemed prior to this corrective action could be exploited to issue SetTokens at a discount. Users redeeming SetTokens in turn would pay a large premium.
The logic implemented in deleverToZeroBorrowBalance() should be introduced in delever() so that if there is a remaining balance, the assets are added as a default position.
diff --git a/contracts/protocol/modules/AaveLeverageModule.sol b/contracts/protocol/modules/AaveLeverageModule.sol
index 97df586..6fce32e 100644
--- a/contracts/protocol/modules/AaveLeverageModule.sol
+++ b/contracts/protocol/modules/AaveLeverageModule.sol
@@ -346,6 +346,13 @@ contract AaveLeverageModule is ModuleBase, ReentrancyGuard, Ownable {
_repayBorrow(deleverInfo.setToken, _repayAsset, repayQuantity);
+ // Update default position first to save gas on editing borrow position
+ _setToken.calculateAndEditDefaultPosition(
+ address(_repayAsset),
+ deleverInfo.setTotalSupply,
+ deleverInfo.preTradeReceiveTokenBalance
+ );
+
_updateLeverPositions(deleverInfo, _repayAsset);
emit LeverageDecreased(Fixed in 70f926f.
AaveLeverageModule.sol#L690, AaveLeverageModule.sol#L706
When an asset is enabled both as a default position and as a borrowing asset, the componentIssueHook() and componentRedeemHook() will trigger twice when calling issue() or redeem(), respectively. This is because DebtIssuanceModule._executeExternalPositionHooks() is called when executing _resolveEquityPositions() and when executing _resolveDebtPositions().
When issuing tokens, the health factor for Aave loans will decrease and open the token up to the possibility of liquidation. The repeated triggers could also prevent users from redeeming their tokens, due to debt position being already repaid or the user having to provide double the allowance needed to proportionally repay the debt.
If a manager was to call deleverToZeroBorrowBalance() the issue would immediately become exploitable. As it would enable an attacker to reduce the health factor of the SetToken by issuing more tokens until they are able to start liquidations. Flash loans could be used to amplify this effect.
To prevent the hooks from triggering twice, the AaveLeverageModule should return early when the _isEquity argument is true. This will ensure that the hooks are only executed for debt positions.
Fixed in 70f926f.
aaveLeverageModule.spec.ts#L477
Although the unit tests provided achieved a very high level of code coverage for the AaveLeverageModule.sol contract and AaveV2.sol library, the lack of proper integration means the overall behavior of the system is not always thoroughly tested. It is likely that the two high-risk issues discovered during the audit would have been identified if tests had been developed which replicate the intended configuration of the system.
In particular, the unit tests made use of the BasicIssuanceModule and not the DebtIssuanceModule as documented in the specification. This resulted in the component, issuance and redeem hooks not triggering for most of the unit tests.
Furthermore, the tests did not include issuance of SetTokens in a leveraged position and did not include any tests for SetToken redemption.
The unit tests for AaveLeverageModule should deploy the DebtIssuanceModule and use it to issue and redeem tokens, instead of the BasicIssuanceModule.
The tests should also be updated to return SetToken to a neutral state at the end of each snapshot, similar to how each test is bootstrapped to thoroughly test the full lifecycle.
Fixed in 99a57dd.
AaveLeverageModule.sol#L188, AaveLeverageModule.sol#L191
The LendingPool and ProtocolDataProvider addresses are stored in the contract; however, Aave strongly recommends that LendingPoolAddressesProvider is used whenever an address is required. Aave's documentation states that LendingPoolAddressesProvider and LendingPoolAddressesProviderRegistry are the only true static addresses.
Aave internally use LendingPoolAddressesProvider whenever cross-contract calls are made, and thus the assumption that addresses for LendingPool and ProtocolDataProvider are immutable might not be valid. However, both contracts are deployed behind upgradeable proxies and the addresses have remained constant for the lifetime of the protocol thus far. The likelihood that the addresses will change is low, but should be mitigated or clearly documented and monitored.
Partially fixed in 70f926f, however, the ProtocolDataProvider identifier was incorrectly encoded and was ultimately resolved in aec8135.
When redeeming SetTokens as the manager's or the protocol's feeRecipient, fees would still apply. This prevented the feeRecipient redeeming all their tokens, as new tokens would get minted in the process.
Without the assistance of an additional module or the manager setting a zero fee, it would not be possible to redeem all underlying components from a SetToken.
Add a check to the calculateTotalFees() function to not charge any fees if the caller is the module's feeRecipient and exclude protocol fees for the protocol's treasury.
The team indicated this in known behavior and that an alternative mechanisms will be used by managers and the protocol treasury to redeem tokens.
Actions to improve the functionality and readability of the codebase are outlined below.
The ProtocolDataProvider is passed as a constructor argument instead of retrieving it via the LendingPoolAddressesProvider. This introduces a small risk of human error and contract address mismatch.
The Aave documentation states that the ProtocolDataProvider for a particular market should be retrieved using LendingPoolAddressesProvider.getAddress(bytes32 id = 0x01).
Fixed in 70f926f.
To save gas, lendingPoolAddressesProvider should be declared as immutable. If lendingPool and protocolDataProvider should also be declared as immutable if it is decided to keep them as static addresses in the contract. This will require minor changes to the constructor to not use state variables that are being assigned, but rather reuse the constructor arguments.
Further reading on immutables can be found here.
Fixed in 70f926f.
When redeeming SetTokens that are in a leveraged position, users are required to provide the necessary assets to proportionally repay the outstanding debt. The documentation for DebtIssuanceModule does not clearly state this requirement. Users should be aware that in order to redeem their SetTokens, they might be required to first obtain necessary tokens from an exchange in order to repay the debt.
Fixed in f2d67d1.
IModuleIssuanceHook interface should be appended to the list of inherited contracts and interfaces.
Fixed in 70f926f.
Spelling and grammar mistakes were identified in the codebase. Fixing these mistakes can help improve the end-user experience by providing clear information on errors encountered, and improve the maintainability and auditability of the codebase.
- AaveLeverageModule.sol#L368: The function reverts if not enough collateral asset is redeemed to buy the required the minimum amount of
_repayAsset. - AaveLeverageModule.sol#L554: Add registration of this module on the debt issuance module for the SetToken.
- AaveLeverageModule.sol#L584: Reverts with "Collateral already enabled" if there are duplicate assets in the passed
_newCollateralAssetsarray. - AaveLeverageModule.sol#L598: Remove collateral assets.
- AaveLeverageModule.sol#L632: Remove borrow assets.
- AaveLeverageModule.sol#L701: the issuance module transfers debt from the issuer. Only callable by a valid module.
Fixed in 70f926f.