Aave DIVA Wrapper

Summary

The AaveDIVAWrapper contract includes a function called addLiquidity, which allows users to add _collateralAmount of liquidity to an existing DIVA Protocol pool identified by the provided _poolId. As part of this process, long position tokens are sent to _longRecipient, while short position tokens are sent to _shortRecipient. Additionally, users can remove liquidity using the removeLiquidity function, which removes liquidity from the specified _poolId by burning an equal amount (_positionTokenAmount) of valid long and short tokens and subsequently transferring the corresponding collateral tokens (e.g., USDC) to the designated _recipient.

To achieve this, AaveDIVAWrapper::addLiquidity internally calls AaveDIVAWrapperCore::_addLiquidity, and AaveDIVAWrapper::removeLiquidity calls AaveDIVAWrapperCore::_removeLiquidity. However, a chronological bug exists between these two functions, potentially leading to a Denial of Service (DoS) for liquidity providers.

In the AaveDIVAWrapperCore::_addLiquidity function, the msg.sender (liquidity provider) has the ability to specify recipients for the position tokens long tokens are sent to _longRecipient, and short tokens to _shortRecipient. While msg.sender initially provides liquidity, they lose ownership of the position tokens upon distribution, as there is no mechanism ensuring that msg.sender retains any reclaimable rights or approvals over them. Consequently, when msg.sender attempts to call removeLiquidity, the transaction fails due to a lack of ownership of the required position tokens, since the recipients (long and short token holders) must approve the transfer or transfer the position tokens back to msg.sender but may refuse to do so.

This vulnerability can effectively lock liquidity in the protocol, preventing providers from reclaiming their collateral and leading to a Denial of Service (DoS) scenario where liquidity providers are unable to exit their positions. Only recipients of the respective position tokens can redeem position tokens.

Vulnerability Details

Code Snippet:

AaveDIVAWrapperCore::_addLiquidity: Reference Link

Code Snippet:

AaveDIVAWrapperCore::_removeLiquidity: Reference Link

It is entirely reasonable for the msg.sender (liquidity provider) to have the ability to specify a recipient when removing liquidity, allowing them to designate who will receive the collateral upon withdrawal. This flexibility ensures that liquidity providers can manage their assets efficiently, directing funds to different addresses as needed.

However, a critical flaw arises from the fact that the liquidity provider does not retain any ownership record or control over the position tokens once they have been distributed to the designated long and short token recipients. Since removing liquidity requires burning an equal amount of long and short position tokens, the liquidity provider becomes entirely dependent on the cooperation of the token recipients to return or approve the use of those tokens.

If these recipients refuse to provide approval or become inactive, the liquidity provider is effectively blocked from reclaiming their collateral, leading to a potential liquidity lock. This oversight introduces a Denial of Service (DoS) risk, where users are unable to exit their positions simply because the recipients of their position tokens do not grant the necessary permissions.

To mitigate this issue, the protocol should implement a mechanism that either ensures liquidity providers maintain some form of reclaimable control over their distributed position tokens or introduces an alternative means for liquidity withdrawal that does not rely entirely on recipient approval. Without such safeguards, liquidity providers risk permanent loss of access to their locked funds, making the system unreliable and vulnerable to manipulation by malicious actors.

Let's analyze the problem with the help of a scenario:

The Problem: Let's assume that

1. Alice provides liquidity and, in return, receives both long and short position tokens.

2. Alice decides to transfer:

   • Long tokens → Bob

   • Short tokens → Charlie

3. Now, Bob and Charlie each own the respective tokens, and they've ownership and control the approval rights over their tokens.

4. Alice wants to remove liquidity, but to do so, she must return an equal amount of long and short tokens to the contract.

5. Bob and Charlie refuse to approve Alice to spend their tokens.

6. Alice is now stuck—she cannot retrieve her original liquidity because she does not have both long and short tokens.

Proof of Concept

To demonstrate this vulnerability, a Proof of Concept (PoC) is provided below.

Note: The PoC is written using the Foundry tool.

1. Step 1: Make a foundry project and put all the contracts in the src directory.

2. Step 2: Create a test folder and mocks folder in the test and src directories respectively (or we already have a mocks folder in src).

3. Step 3: Create all necessary mocks

4. step 4: Create a test file with any name in the test directory.

1. Step 5: Now, Put the following Test PoC in the test file adjacent to the setUp function.

1. Step 6: Now, to run the test, run the following commands in the terminal in sequence:

1. Step 7: See the output of the test in the terminal, the output should be similar to the below one:

As demonstrated in the output, test passes and removeLiquidity function reverts with error ERC20InsufficientAllowance.

Impact

• Why This is an Issue?

  • Denial of Service (DoS) Risk: Alice cannot remove liquidity unless she somehow gets back both position tokens.

  • Trust Assumption: The system assumes Bob and Charlie will cooperate, which is not guaranteed. They can be eloped anytime

  • Funds at Risk: Alice might never be able to reclaim her collateral due to malicious behavior or even simple negligence by Bob or Charlie.

• Why This is a Medium Issue?

  • The protocol provides a function to redeem position tokens: AaveDIVAWrapper::redeemPositionToken, which internally calls AaveDIVAWrapperCore::_redeemPositionToken. This function allows position token holders to redeem collateral independently, reducing the severity of the issue.

  • The core problem lies in the trustworthiness of the long and short token recipients. Since the liquidity provider loses direct ownership of position tokens upon transferring them, they become fully dependent on the recipients’ cooperation to approve transfers when attempting to remove liquidity.

  • However, this issue does not result in an immediate or permanent loss of funds—the liquidity provider’s collateral is still retrievable as long as the token recipients act in good faith. The risk is primarily one of accessibility and trust, rather than outright loss or protocol failure.

  • Due to the existence of the redeemPositionToken function and the fact that the issue stems from user behavior rather than a direct protocol vulnerability, this should be classified as a Medium-severity issue rather than a High or Critical one.

  • If the protocol is designed to delegate the responsibility of removing liquidity to the recipients of long and short position tokens, the removeLiquidity function still does not work as intended. The issue arises because if the long and short token recipients are different entities, neither of them will possess both token types. Since removing liquidity requires burning an equal amount of long and short tokens, the transaction will fail due to an insufficient balance of either token.

  • Furthermore, even if the function does not revert due to a zero-value transfer, it will not produce the expected outcome, as the fundamental requirement of having both long and short tokens is not met.

  Code Reference: AaveDIVAWrapperCore::_removeLiquidity

  ...

  ​

  uint256 _userBalanceShort = _shortTokenContract.balanceOf(msg.sender);

  uint256 _userBalanceLong = _longTokenContract.balanceOf(msg.sender);

  uint256 _positionTokenAmountToRemove = _positionTokenAmount;

  ​

  if (_positionTokenAmount == type(uint256).max) {

  @> // if msg.sender doesn't have both tokens amount then either of tokens

  // balance will be zero and that will be chosen to tranfer

  @> _positionTokenAmountToRemove = _userBalanceShort > _userBalanceLong

  ? _userBalanceLong

  : _userBalanceShort;

  }

  ​

  // Transfer short and long tokens from user to this contract. Requires prior user approval on both tokens.

  // No need to use `safeTransferFrom` here as short and long tokens in DIVA Protocol are standard ERC20 tokens

  // using OpenZeppelin's ERC20 implementation.

  _shortTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);

  _longTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);

  ​

  ...

  • Key Issue: The removeLiquidity function only works when the recipient of both long and short tokens is the same entity—either the liquidity provider themselves or a single recipient holding both token types. If different recipients receive the long and short tokens separately, the function becomes non-functional, as neither party possesses the necessary balance to execute the transfer.

  • Since removeLiquidity is a crucial function that reverses the effects of addLiquidity, it must be fully functional and reliable. Therefore, it is essential to resolve this oversight by implementing a mechanism that ensures liquidity can always be removed, regardless of how position tokens are distributed.

Tools Used

• Manual Review

• Foundry

Recommendations

To mitigate this issue, a few possible solutions exist:

• Implement an "Atomic Transfer Mechanism"

  • Instead of directly sending position tokens to different recipients, implement a single atomic transaction that guarantees Alice can retrieve her liquidity.

  Example: The addLiquidity function could include an internal accounting system where Alice retains a right to reclaim tokens instead of immediately transferring ownership.

• Approval to recipients

  • The Liquidity Provider may authorize Position Token recipients to spend Position Tokens on their behalf. Additionally, there should be a confirmation mechanism that verifies if the Liquidity Provider has sufficient Position Tokens. If not, the logic should check whether the recipients have already redeemed Position Tokens. If they have, the system should revert the transaction with an appropriate message.

• Create an "Emergency Exit" Function

  • If Liquidity Provider is unable to retrieve their tokens, the protocol could allow an alternative withdrawal mechanism:

  • After a timelock or governance approval, Liquidity Provider could forcibly reclaim an equivalent value of collateral and a forcibly forfeiture from recipients approval.

  • This would prevent funds from being permanently locked.

• Set msg.sender as both the longTokenRecipient and shortTokenRecipient.

  • Since both long and short position tokens are ERC20-based tokens, it is logical to assign msg.sender as the initial recipient. When an ERC20 token is owned by a user, they have full control over transferring or distributing it as they see fit. This flexibility eliminates the need to pre-define separate recipients at the time of adding liquidity.

  • By ensuring that msg.sender initially receives both position tokens, the liquidity provider retains complete ownership and control, allowing them to later transfer or allocate the tokens to different recipients at their discretion.

    • Why is this important?

    • If different recipients are pre-selected for long and short tokens, it immediately removes control from the liquidity provider, potentially leading to liquidity lock scenarios where they are unable to remove liquidity if the recipients refuse to return the tokens or approve transfers.

    • By assigning both position tokens to msg.sender by default, the liquidity provider retains ownership and can manage their assets efficiently without any unintended restrictions.