Felix 2nd Audit audit report

ABSTRACT

Dedaub was commissioned to conduct a second security audit of the Felix protocol. Felix is a fork of Liquity V2, adapted for deployment on the HyperLiquid blockchain.

BACKGROUND

The first Felix audit focused on several significant modifications to Liquity V2, including the adoption of a Transparent Proxy design for each contract, making critical parameters mutable, centralizing administrative actions, and removing the gas compensation mechanism.

This audit examines the following modifications to the protocol:

Certain functions have been rewritten in assembly to address maximum contract size limitations in the TroveManager and BorrowerOperations contracts.
The AdminController contract now employs a Role-Based Access Control (RBAC) model instead of single ownership, allowing authority for operations to be distributed across multiple multisigs.
A new feature has been implemented in the AdminController to enable arbitrary shutdown of a branch and the resumption of a previously shutdown branch. The shutdown may occur manually or automatically due to an oracle failure or due to a lower than required system collateralization ratio.

The audit also examined the protocol’s mainnet deployment script.

SETTING & CAVEATS

This audit report mainly covers the changes of the at-the-time private repository https://github.com/felixprotocol/felix-contracts (branch feat/felixRenaming) of the Felix Protocol between commit 3605746bb372ff8fbeb05205cc4652d414c5ebfb and commit 377ba78b432317ff805ae042affd2ca8cf753420.

Two auditors (and a junior auditor) worked on the codebase for 5 days on the following contracts:

src//
├── ActivePool.sol
├── AddressesRegistry.sol
├── AdminController.sol
├── BorrowerOperations.sol
├── CollateralRegistry.sol
├── CollSurplusPool.sol
├── DefaultPool.sol
├── Dependencies/
│   ├── AddRemoveManagers.sol
│   ├── Constants.sol
│   └── LiquityBase.sol
├── FelixToken.sol
├── GasPool.sol
├── HintHelpers.sol
├── Libraries/
│   ├── BorrowerOperationsInit.sol
│   ├── LiquityBaseInit.sol
│   └── TroveManagerInit.sol
├── MultiTroveGetter.sol
├── PriceFeeds/
│   └── HLPriceFeed.sol
├── SortedTroves.sol
├── scripts/
│   └── Mainnet/DeployFelixMainnet.sol
├── StabilityPool.sol
├── TroveManager.sol
├── TroveNFT.sol
└── Zappers/
    ├── BaseZapper.sol
    ├── GasCompZapper.sol
    └── Interfaces/
        ├── LeftoversSweep.sol
        ├── LeverageLSTZapper.sol
        ├── LeverageWETHZapper.sol
        └── Modules/
            └── WETHZapper.sol

The audit’s main target is security threats, i.e., what the community understanding would likely call “hacking”, rather than the regular use of the protocol. Functional correctness (i.e. issues in “regular use”) is a secondary consideration. Typically it can only be covered if we are provided with unambiguous (i.e. full-detail) specifications of what is the expected, correct behavior. In terms of functional correctness, we often trusted the code’s calculations and interactions, in the absence of any other specification. Functional correctness relative to low-level calculations (including units, scaling and quantities returned from external protocols) is generally most effectively done through thorough testing rather than human auditing.

PROTOCOL-LEVEL CONSIDERATIONS

Manual branch shutdown and resumption risks

Protocol Level Consideration | Status: INFO

Felix introduces admin functionality to manually shut down and resume a branch’s operation, a feature that entails several risks. In the original Liquity v2, shutdown is considered a terminal branch state from which recovery is not possible, as the protocol was not necessarily designed to support resumption. A branch may be shut down due to a broken price oracle or if the branch collateral ratio drops below the designated shutdown collateral ratio. To mitigate risks, the protocol incentivizes Felix redemptions to rapidly reduce the branch’s debt and prevent the accumulation of bad debt, which could jeopardize the stability of the stablecoin.

Since the original Liquity v2 was not designed with branch resumption in mind, and given the protocol’s significant complexity with numerous invariants required to ensure secure and robust operation, we recommend extensive testing and potentially financial modeling before deploying this feature. The contained scope of the audit, coupled with the protocol’s complexity, does not allow for extensive testing and modelling, which could provide definitive answers on the security and robustness of the solution, but revealed several issues (C1, M1, M2) related to branch resumption that could jeopardize the protocol’s functionality and stability by affecting accounting and disrupting protocol incentives.

Manual branch resumption could be a challenging and risky decision for the protocol administrator. Not only must the reason for the shutdown be addressed, but several other factors have to also be considered to ensure a smooth and safe resumption. The centralization risks and related concerns are discussed in further detail in issue N1.

Additionally, the protocol’s deployment on Hyperliquid L1 could complicate branch shutdown and resumption decisions, as this new ecosystem/market may not be sufficiently efficient. As a result, the incentives set by the protocol may not always function as expected. For example, in the event of a collateral price collapse or a broken oracle, the market might not react swiftly enough to liquidate and redeem troves, potentially leading to significant bad debt and reducing the likelihood of a successful branch resumption.

Collateral token risks

Protocol Level Consideration | Status: INFO

Felix plans to use the HYPE (Hyperliquid) token (and possibly other non-standard collateral tokens) as the collateral for one of its branches. However, this decision carries the inherent risk that the HYPE token may lack sufficient liquidity and a highly efficient market, making its price susceptible to manipulation. Furthermore, since HYPE is not widely supported by major centralized exchanges, the Hyperliquid price oracle cannot leverage external exchanges to source its price, a practice that is generally followed for other assets according to the Hyperliquid docs. As a result, the token’s price is determined exclusively by activity within the Hyperliquid market, which could amplify the risk of price distortions in scenarios of low trading volume or targeted manipulation. The Felix administrators should carefully determine the minimum safe individual trove and branch collateralization ratios to ensure the protocol remains robust, even during periods of significant collateral price volatility. Of course, Felix users should understand that collateral risks can never be entirely eliminated.

Risks associated with Hyperliquid’s EVM

Protocol Level Consideration | Status: INFO

Felix plans to launch in conjunction with the mainnet release of the Hyperliquid EVM, which is currently in testnet, a decision that carries inherent risks. At the time of the mainnet release, the market is likely to experience inefficiencies, such as inefficient liquidations and redemptions, as participants are still experimenting with the new environment.

Additionally, the blockchain may face potential challenges, including latency or liveness issues, which could affect its overall performance. Another area of concern is related to oracles, as price data will be supplied directly by Hyperliquid validators. This reliance could introduce risks if validators encounter difficulties in providing accurate or timely price information.

VULNERABILITIES & FUNCTIONAL ISSUES

This section details issues affecting the functionality of the contract. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:

Issue resolution includes “dismissed” or “acknowledged” but no action taken, by the client, or “resolved”, per the auditors.

CRITICAL SEVERITY

Incorrect aggregate debt calculation and minting after branch resumption

Critical | Status: RESOLVED

In Felix, simple (non-compounding) interest accrues on each trove’s debt continuously, and gets compounded discretely every time a trove is touched. Aggregate (branch) accrued Trove debt is periodically (as part of certain user actions) minted as Felix. Two variables, aggRecordedDebt and aggWeightedDebtSum, in the ActivePool contract are employed to track the aggregate recorded debt and the sum of individual recorded Trove debts weighted by their respective chosen interest rates. There is also ActivePool::lastAggUpdateTime that tracks the last time at which the aggregate recorded debt and weighted sum were updated.

The calculation of pending aggregate interest returns early with 0 if shutdownTime != 0 as aggregate interest is not minted or accrued after shutdown.

ActivePool::calcPendingAggInterest():122

function calcPendingAggInterest() public view returns (uint256) {
  if (shutdownTime != 0) return 0;

  // We use the ceiling of the division here to ensure positive error,
  // while we use regular floor division when calculating the interest
  // accrued by individual Troves.
  // This ensures that `system debt >= sum(trove debt)` always holds, and
  // thus system debt won't turn negative even if all Trove debt is
  // repaid. The difference should be small and it should scale with the
  // number of interest minting events.
  return Math.ceilDiv(aggWeightedDebtSum *

    (block.timestamp - lastAggUpdateTime), ONE_YEAR * DECIMAL_PRECISION);

}

If the branch is resumed after a shutdown, the update logic in ActivePool::_mintAggInterest may lead to incorrect aggregate debt calculations. Specifically, this function sets lastAggUpdateTime to block.timestamp, which could be later than shutdownTime. In such a scenario, the period between shutdownTime and lastAggUpdateTime would not be accounted for in the aggregate (branch) debt calculation.

As a result, the aggregate debt trackers would not be updated correctly, leading to insufficient minting of Felix debt. Meanwhile, the individual troves’ debt would continue to accrue normally unless a trove is touched during shutdown (related issue M1). This mismatch could cause the total individual trove debt to exceed ActivePool::aggRecordedDebt, potentially resulting in calculation underflows during significant Felix debt repayments.

ActivePool::_mintAggInterest is invoked during the processes of closing, liquidating, and urgently redeeming a trove. Since it is callable during shutdown, the associated issues and risks described should be carefully considered, as they might even lead to the branch getting stuck.

HIGH SEVERITY

    [No high severity issues]

MEDIUM SEVERITY

Borrowers can reset interest period during shutdown

Medium | Status: RESOLVED

A trove accumulates interest, which eventually is debited to the trove. Calculating the interest a borrower owes relies on calculating the period for which the interest applies. As seen below, any time after the shutdownTime is not accounted for in the interest calculation.

TroveManager::_getInterestPeriod():LoC

function _getInterestPeriod(uint256 _lastDebtUpdateTime)
  internal view returns (uint256)
{
  if (shutdownTime == 0) {
    // If branch is not shut down, interest is earned up to now.
    return block.timestamp - _lastDebtUpdateTime;

  } else if (shutdownTime > 0 && _lastDebtUpdateTime < shutdownTime) {
    // If branch is shut down and the Trove was not updated since
    // shut down, interest is earned up to the shutdown time.
    return shutdownTime - _lastDebtUpdateTime;

  } else {
    // if (shutdownTime > 0 && _lastDebtUpdateTime >= shutdownTime)
    // If branch is shut down and the Trove was updated after shutdown,
    // no interest is earned since.
    return 0;
  }
}

During branch shutdown, users with Felix balance may redeem collateral from a specified set of troves, through TroveManager::urgentRedemption(). Once a trove is redeemed from, its corresponding lastDebtUpdateTime, is updated to reflect the value of block.timestamp at the time of redemption, which will be greater than shutdownTime.

This means that in case of branch resumption the calculator of the interest period will differ between troves that were redeemed from during shutdown and troves that were not.

Let’s say there is a trove T in a branch that is not shutdown. We denote the lastDebtUpdateTime of T before shutdown as t. Shutdown happens at a time t + k. T is not redeemed from during shutdown. The branch is resumed at time t + k + l. Now, if the trove data is updated immediately after resuming, the interest period for trove T will be calculated from time (t + k + l) - t = k + l.

Suppose instead that a small amount was redeemed from the trove during shutdown, at a time t + k + m (where m < l), such that the trove remains not stale. When this happens, debt corresponding to the time period (t + k) - t = k is accrued. Then, when the trove is redeemed from, after resuming the branch at time (t + k + m + n), as before (where l = m + n), the interest period will be calculated as (t + k + m + n) - (t + k + m) = n. The total interest period considered for interest accrual is then k + n. Note that n is smaller than l, since l = m + n.

In Liquity V2 trove redemptions are incentivised during shutdown, to clear any branch debt as fast as possible. However, here the introduction of a possible branch resumption incentivises the trove owners to redeem small values during shutdown to avoid paying interest fees for the shutdown period once the branch resumes. In this scenario, troves that are not redeemed from during temporary shutdown are penalised.

Potential griefing attack via small debt troves exploited during shutdown

Medium | Status: RESOLVED

The function TroveManager::redeemCollateral marks troves with debt below MIN_DEBT as zombies, removing them from the sorted troves list to prevent clogging future regular redemptions. In contrast, the function urgentRedemption, callable only during a shutdown, does not zombify troves. This is because, in Liquity v2, the shutdown state is terminal, and troves can be selected for redemption by their ID rather than traversing the sorted troves list, thus there is no risk of a clogged sorted troves list during shutdown.

An adversary could exploit this discrepancy by creating numerous troves with debt below MIN_DEBT through selective redemptions during a shutdown. Upon branch resumption, redemption calls (restricted to redeemCollateral in non-shutdown mode) would be forced to iterate over all these small (or zero-debt) troves before reaching a trove with a meaningful amount of Felix debt. This would require the caller to allocate more gas and specify a larger number of troves to iterate over than usual, effectively exposing them to a griefing attack. The likelihood of such an attack varies based on system variables such as the profitability of redemptions and the potential for an attacker to block redemptions for their own benefit (e.g., avoiding being redeemed). Its success depends on the blockchain’s gas costs.

LOW SEVERITY

No validation of collateral deployment params

Low | Status: OPEN

The initialization of collateral params is clearly crucial for the correct deployment of the protocol. However, the deployment script reads these params in a hacky and error-prone manner, which could lead to unnoticed errors.

Firstly, using Collaterals.COLLATERALS_LENGTH to determine the size of the Collaterals enum is unnecessarily hacky. If the enum is accidentally reordered the value of COLLATERALS_LENGTH might not match the size. One could more safely use uint8(type(Collaterals).max)+1.

More importantly, the script implicitly assumes that the Collaterals enum corresponds exactly to the contents of the trove-manager-params.json file, but this correspondence is not validated. If the json file has more entries than the enum, the extra entries will be silently ignored. Moreover, if the entries of the json array are re-ordered, the params will be assigned to the wrong collateral, since the script assumes, for instance, that WBTC is always at index 1.

To prevent these issues, we recommend implementing proper validation of the JSON file. Specifically:

Ensure the JSON file’s length matches the size of the Collaterals enum.
Require each JSON entry to include the collateral name.
Validate that the order of entries in the JSON file matches the order in the enum.

CENTRALIZATION ISSUES

It is often desirable for DeFi protocols to assume no trust in a central authority, including the protocol’s owner. Even if the owner is reputable, users are more likely to engage with a protocol that guarantees no catastrophic failure even in the case the owner gets hacked/compromised. We list issues of this kind below. (These issues should be considered in the context of usage/deployment, as they are not uncommon. Several high-profile, high-value protocols have significant centralization threats.)

Centralized branch shutdown and resumption risks

Centralization | Status: INFO

A branch can be shut down in three ways: automatically due to an oracle failure, when TCR < SCR, or directly by the admin. Similarly, branches can be manually resumed by the admin, introducing an element of centralization and potential risk if the resumption is mishandled.

In case a branch has been shut down due to oracle failure, most protocol functions are disabled, and HLPriceFeed::fetchPrice(), returns the lastGoodPrice, i.e. the last valid price returned by the oracle. The admin is in control of resuming the branch, and should only do so after updating the priceFeed contract to a functional one. In the current version of the code, there is no logic that ensures such an update has been performed when executing a branch resumption.

If a branch is shut down due to TCR < SCR and an oracle failure subsequently occurs, the HLPriceFeed’s call to BorrowerOperations::shutdownFromOracleFailure will terminate early without emitting the ShutDownFromOracleFailure event. If the monitoring system fails to detect the oracle failure, the admin might resume branch operations without addressing the failing oracle issue, which would lead to another shutdown. It might be worth considering an update to HLPriceFeed::_fetchPrice to emit an event when the oracle returns an invalid price, as this could simplify the monitoring of oracle failures. The same issue might arise in case the branch has been manually shut down by the admin.

Resuming the branch under the unfavorable conditions entails significant risks and thus resumption should be approached with careful consideration of the branch’s overall state and health. Key concerns include:

Total System Collateralization Ratio and Bad Debt: The overall collateralization ratio and the accumulation of bad debt must be assessed to ensure the branch’s and whole protocol’s financial stability.
Stability of Felix’s Price: The stability and reliability of Felix’s price during resumption are critical for maintaining user trust and protocol integrity.
Stability of Collateral Prices: Fluctuations in collateral prices can impact the branch’s solvency and should be carefully evaluated.
Oracle Reliability: The trustworthiness and functionality of the oracle must be verified, as its accuracy is crucial for operations.

Resumption should proceed only after thorough checks on these and potentially other factors to mitigate risks and ensure a smooth recovery.

Upgradeable contracts

Centralization | Status: INFO

The Felix protocol, unlike Liquity v2, permits contract upgrades while maintaining the existing state through the use of transparent proxies. This design enables essential upgrades to the contracts’ logic but also introduces risks. Malicious actors, whether protocol owners or adversaries compromising the owners’ accounts, could exploit this feature and introduce malicious code. Additionally, protocol owners might unintentionally introduce buggy functionality during upgrades. The AdminController contract, which facilitates updates, enforces a timelock period (currently set to 3 days) for each contract upgrade. While this provides users with a reasonable window to react, it could still be easily overlooked by a significant number of users.

Mutable critical parameters

Centralization | Status: INFO

The Felix protocol introduces mutability for the following critical branch parameters:

MCR
CCR
SPYield
InterestRouter
PriceFeed
MaxDebtCap

Incorrect or malicious parameter settings can negatively affect the health of a branch or the entire system due to the accrual of excessive bad debt, particularly in inefficient markets. Protocol owners should set these parameters defensively to ensure the stability, security, and resilience of the protocol.

The owners of the protocol can add new collaterals/branches. We believe that choosing collaterals requires careful consideration, as the collapse of a branch (whether due to collateral collapses or branch oracle fails) could create bad debt that is shared across all branches. This, in turn, could lead to bank runs, as described in the known Liquity v2 issues guide.

OTHER / ADVISORY ISSUES

This section details issues that are not thought to directly affect the functionality of the project, but we recommend considering them.

Inconsistent CCR and MCR bounds between contracts

Advisory | Status: INFO

The AddressesRegistry defines the bounds for CCR on line 95 as _ccr <= 1e18 || _ccr >= 4.5e18. However, AdminController only allows proposals for CCR that are < 1e18, and > 4.5e18 (line 475). Similarly for MCR (see AddressesRegistry::96 and AdminController:455).

Wrong import path

Advisory | Status: INFO

DeployAndOpenTroves.sol::6 imports IBorrowerOperations from inside the folder ../../interfaces, this should be ../../Interfaces.

DISCLAIMER

The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.

ABOUT DEDAUB

Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub’s auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.

SUMMARY

ABSTRACT

BACKGROUND

SETTING & CAVEATS

PROTOCOL-LEVEL CONSIDERATIONS

Manual branch shutdown and resumption risks

Collateral token risks

Risks associated with Hyperliquid’s EVM

VULNERABILITIES & FUNCTIONAL ISSUES

CRITICAL SEVERITY

Incorrect aggregate debt calculation and minting after branch resumption

HIGH SEVERITY

MEDIUM SEVERITY

Borrowers can reset interest period during shutdown

Potential griefing attack via small debt troves exploited during shutdown

LOW SEVERITY

No validation of collateral deployment params

CENTRALIZATION ISSUES

Centralized branch shutdown and resumption risks

Upgradeable contracts

Mutable critical parameters

OTHER / ADVISORY ISSUES

Inconsistent CCR and MCR bounds between contracts

Wrong import path

DISCLAIMER

ABOUT DEDAUB