Quantstamp reviewed Levva Protocol on 11 Oct 2023: 24 findings, 0 critical; 16 marked fixed, 8 acknowledged.
Report hosted on certificate.quantstamp.com.
Trustblock does not review code.
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 0 | 0 | 5 |
| Medium | 0 | 0 | 3 |
| Low | 0 | 8 | 8 |
In scenarios with system insolvency, i.e., the total collateral value is insufficient to support the entire debt value on the opposite side, the protocol admin will invoke `MarginlyPool.shutdown()`. According to the [official documentation](https://docs.marginly.com/protocol-mechanics/risk-management#shutdown-mode): > In the shutdown mode, no new positions are allowed to be opened, and the side opposite to the insolvent side is responsible for restoring the system's solvency at its expense. However, according to the code implementation, users can still open new positions even if the pool is in shutdown mode, as the code does not explicitly prevent the users from doing so.
In the `MaxBinaryHeapLib.remove()` function, the heap replaces the to-be-removed node with the last node (i.e., the node with the largest index) and invokes `heapifyDown()` with the inserted node to adjust its position in the heap. However, the inserted node may need to be pushed toward the root by calling `heapifyUp()` instead. In such cases, the inserted node will not be placed in the correct position in the heap, which could lead to a failure in maintaining the maximum node in subsequent operations. The max heap aims to track the pool's riskiest short or long position. When the `reinit()` function is invoked, it only evaluates whether the position at the top of the heap is unhealthy, and if so, the position will get liquidated. As a result, the failure of the heap could prevent the liquidation of an unhealthy position if a healthy position is at the top of the heap. Note that liquidators can still trigger the liquidation of an unhealthy position by directly calling `receivePosition()`. However, if the liquidators are absent, the system may be unable to execute the liquidation by itself due to this bug.
In the `MarginlyPool` contract, the `receivePosition()` function allows users to receive unhealthy positions while choosing the amount of debt to repay (and the amount of collateral to add) to restore position health. This flexibility can result in changes to the position's leverage. However, since no `updateHeap()` is called at the end of the `receivePosition()` function, the `sortKey` of the user's position remains unchanged, even when the position leverage decreases. As a result, the position is incorrectly positioned in the heap, potentially preventing other unhealthy positions from being liquidated by `reinit()` due to the inconsistent sorting.
In case the collateral of the long or short position is ever exceeded by their respective debt, the pool can be shut down from its regular use and put into a so-called emergency mode by a privileged role. A short emergency, for example, happens when the base debt exceeds the quote collateral. As a result, all quote tokens of the protocol are sold off and, together with the remaining base assets of the protocol, will be withdrawable by all long positions and all base asset lenders. The withdrawable amount for the pool's leftover funds of each position is essentially proportional to the position's share of base collateral in relation to the pool's total base collateral. As leveraged long positions have a very high `discountedBaseAmount`, as their leveraged base ownership is recorded in that field, their proportional share is up to 20x (i.e. max leverage) higher than for base lenders or long positions without leverage. It is our opinion that the emergency fund distribution should be based on the collateral deposited to the protocol and not based on possibly leveraged amounts. The fairness is also not the only concern here. The emergency mode activation e.g. a short emergency could be front-run by a max leverage long position that sells off all remaining quote collateral (without the `MarginlyParams.quoteLimit` being exceeded for the base token) that could manage to get a great share of the whole protocol's funds that way. It should also be mentioned that due to all quote collateral being sold off and made accessible to the base lenders and longers, even the base lenders or the unleveraged longs could profit from such emergencies (specifically if `emergencyCoefficient > 1`).
Marginly pools execute swaps through the router when a user creates a long or short position, closes their position, or their position is liquidated. The pool calculates a minimum output or maximum input token amount based on the spot price of a Uniswap V3 pool. The router then enforces that the actual received or input amount falls within the limits. However, slippage checks based on spot prices of Uniswap V3 pools may be ineffective, as the price impact of a user’s trade can be relatively small when the pool has enough liquidity. In other words, the larger the pool’s liquidity, the less significant the price impact tends to be. This ineffectiveness is exacerbated by Uniswap V3’s centralized liquidity design, where specific tick ranges can have particularly deep liquidity, further reducing the potential price impact of the user’s trade. As the slippage checks are ineffective in protecting users from significant price changes, the user may suffer from sandwich attacks. In such attacks, the attacker manipulates token balances in a Uniswap V3 pool, thereby influencing the outcomes of swaps. As a result, users may receive fewer output tokens or may be required to transfer more input tokens to the pool than expected. The possible consequences in each scenario are: - Open position: Users might have a position with a higher leverage level than intended. - Close position: Users may receive fewer tokens than expected. - Liquidation: The swapped amount may fall short of covering the debt, causing the lenders to absorb the losses instead of resulting in profit.
If a user wants to keep their leveraged long or short position from being liquidated, they might need to deposit additional collateral to lower the position's leverage. Before the collateral deposit is accepted, it is checked that the pool's total collateral value (denominated in the quote asset price), together with this deposit, does not exceed `MarginlyParams.quoteLimit`. Else, the transaction reverts. This opens up the possibility of a DoS-attack vector, wherein a malicious actor possessing substantial financial resources strategically deposits both the quote and base assets as a lending position into the protocol with the goal of elevating the protocol's asset balances for both the quote and base assets to reach `MarginlyParams.quoteLimit`. Unless profitable positions are closed, or liquidations occur, these values will not change, making the active position's leverage unadjustable. As most liquidations absorbed by the pool will profit collateral holders, there is an incentive for malicious actors to perform such an attack. This could also happen in more natural, non-malignant conditions: With a falling base asset price, many shorts would close, increasing the base balance in the protocol. If such closing positions reach the quote limit, long positions cannot keep their positions healthy.
In the event of a short emergency within a Marginly ETH/USDC pool, where the total ETH debt value exceeds the total USDC collateral value, the protocol admin initiates a pool shutdown and sets the mode to `ShortEmergency` by invoking the `setEmergencyMode()` function. During the execution of this function, all USDC collateral minus the USDC debt is swapped into ETH using the Marginly router. However, the swap is executed with the minimum output token amount set to 0, which makes the swap susceptible to sandwich attacks. In the most severe scenario, the pool may receive nearly 0 ETH in return, with most of the value extracted by the malicious attacker.
The Marginly Pool can hold multiple to-be-liquidated positions at once. If protocol usage is slow and bots are not participating in liquidations via `receivePosition()`, the total debt of one position's side can, therefore, briefly exceed the collateral of a position by a factor greater than the max leverage. This is especially note-worthy in situations where the pool would be about to shut down, i.e., where the base debt exceeds the quote collateral. If such a shutdown call would be front-run by a user calling `receivePosition()` on a few liquidatable short positions (i.e., increasing quote collateral), they could perform enough liquidations to stop the short-emergency from being activated. In such a scenario, the system leverage could be almost arbitrarily high (assuming, e.g., base debt is equal to a hundred thousand USD and quote collateral equal to a hundred thousand and one USD would update the system short leverage to a hundred thousand), making the interest rate for shorts for the period possibly vastly higher than anticipated. The conditions for this would, however, be improbable to happen. Not only would the pool need to be close to an emergency state, but an attacker would need to be perfectly organized to profit from such an attack (i.e., have a massive long position in place to receive the fees and front-run the emergency activation with a perfectly orchestrated series of `receivePosition()` calls).
Emitting events is a good practice to validate the proper deployment and initialization of the contracts. Also, any important state transitions can be logged, which is beneficial for monitoring the contract and tracking eventual bugs or hacks. Below, we present a non-exhaustive list of events that could be emitted to improve application management: - `FullMarginlyFactory.changeSwapRouter()` should emit the new swap router address. - `MarginlyFactory.changeSwapRouter()` should emit the new swap router address. - `MarginlyPool.setParameters()` should emit the global pool parameters. - `RouterStorage.addDexAdapter()` should emit the address of the DEX adapter added. - `AdapterStorage.addPools()` should emit the pool address added.
The clone-and-own approach involves copying and adjusting open-source code at one's discretion. From the development perspective, it is initially beneficial as it reduces the amount of effort. However, from the security perspective, it involves some risks as the code may not follow the best practices, contain a security vulnerability, or include intentionally or unintentionally modified upstream libraries. `OracleLib.sol` is copied, and its Solidity version is upgraded to `0.8.x`, allowing it to be used with the other contracts. Also, an `unchecked` block is added to the `getSqrtRatioAtTick()` function, as the arithmetic operations were performed without an overflow check in the previous Solidity version.
The owner of the factory contract can call `setOwner()`, and the owner of the `AdapterStorage` and `RouterStorage` can call `transferOwnership()` to transfer the ownership to a new address. Suppose an uncontrollable address is accidentally provided as the new owner's address. In that case, the contract will no longer have an active owner, and functions restricted to the contract owner can no longer be executed. - `FullMarginlyFactory` and `MarginlyFactory` have a custom owner implementation and use `setOwner()` to transfer the ownership, which does not follow the two-step pattern. - `AdapterStorage` and `RouterStorage` inherit OpenZeppelin's `Ownable` contract.
It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. Specifically, in the following functions, arguments of type `address` may be initialized with value `0x0`: - `FullMarginlyFactory.constructor()`: - `uniswapFactory` - `swapRouter` - `feeHolder` - `WETH9` - `techPositionOwner` - `FullMarginlyFactory.setOwner()` - `owner` - `MarginlyPool._initializeMarginlyPool()` - `quoteToken` - `baseToken` - `quoteTokenIsToken0` - `uniswapPool` - `params` - `MarginlyPool.setParameters()` - `params`
The owner can set arbitrary fees via the function `MarginlyPool.setParameters()`. Specifically, the owner can set the following fees and rates up to and exceeding 100%. - `MarginlyPool.params.interestRate` can be set above 100% (1,000,000). - `MarginlyPool.params.fee` can be set above 100% (1,000,000). - `MarginlyPool.params.swapFee` can be set above 100% (1,000,000). - `MarginlyPool.params.positionSlippage` can be set above 100% (1,000,000). - `MarginlyPool.params.mcSlippage` can be set above 100% (1,000,000).
If the owner renounces ownership, all ownable contracts will be left without an owner. Consequently, any function guarded by the `onlyOwner` modifier will no longer be able to be executed. The functions listed under _Privileged Roles and Ownership_ will no longer be executable if contract ownership is renounced.
The code relies on the function `SafeERC20.safeApprove()`. This function has been deprecated and shall not be used instead of `approve()` since 1) it can still be front-run, 2) it uses additional gas, and 3) it does not work with tokens that are not ERC-20 compliant. For more information, see [this issue](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2219). - `MarginlyKeeper.liquidateAndTakeProfit()` uses `IERC20.approve()` and `SafeERC20.safeApprove()`, which is generally not recommended. - The other files use Uniswap's `TransferHelper.safeApprove()`, which still has the issue of being unable to change a currently active approval. Some places in the code call `TransferHelper.safeApprove(0)` to solve this issue, but some don't.
Every Solidity file specifies a version number of the format `pragma solidity (^)0.8.*` in the header. The caret (`^`) before the version number implies an unlocked pragma, meaning the compiler will use the specified version _and above_, hence the term "unlocked".
The report names a repository or files. Open the report to see its scope.
Repository and files the report names (40)
236 projects reviewed · 332 reports · latest Oct 2026
See Quantstamp’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us