Aave DIVA Wrapper
DIVA
HardhatDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Inconsistency Between addLiquidity and removeLiquidity in Handling Recipients of long and short position tokens

oluwaseyisekoni

Summary

The _addLiquidity function allows the caller to specify different addresses for receiving the long and short position tokens (_longRecipient and _shortRecipient). This means that a liquidity provider can add collateral and have the long and short tokens sent to different addresses based on a custom use case or agreement.

However, _removeLiquidity enforces that both long and short tokens must be transferred from a single msg.sender.

Vulnerability Details

_addLiquidity::AaveDivaWrapperCore.sol

function _addLiquidity(
bytes32 _poolId,
uint256 _collateralAmount,
address _longRecipient,
address _shortRecipient
) internal {
// Verify that the collateral token used in the DIVA Protocol pool corresponds to a registered
// collateral token in the AaveDIVAWrapper contract. Returns zero address if the wToken is not registered.
IDIVA.Pool memory _pool = IDIVA(_diva).getPoolParameters(_poolId);
address _collateralToken = _wTokenToCollateralToken[_pool.collateralToken];
​
// Confirm that the collateral token is registered. This check is performed early
// to ensure an immediate and graceful revert rather than allowing execution to continue until the `mint`
// operation at the end of the `_handleTokenOperations` function, which would then fail when attempting to call
// the `mint` function on address(0).
if (_collateralToken == address(0)) {
revert CollateralTokenNotRegistered();
}
​
// Transfer collateral token from caller to this contract, supply to Aave, and mint wTokens
// to this contract.
_handleTokenOperations(_collateralToken, _collateralAmount, _pool.collateralToken);
​
// Add liquidity to the DIVA Protocol pool associated with the provided `_poolId`
// using the wToken and send the position tokens to the provided recipients.
@> IDIVA(_diva).addLiquidity(_poolId, _collateralAmount, _longRecipient, _shortRecipient);
}

_removeLiquidity::AaveDivaWrapperCore.sol

function _removeLiquidity(
bytes32 _poolId,
uint256 _positionTokenAmount,
address _recipient
) internal returns (uint256) {
// Query pool parameters to obtain the collateral token as well as the
// short and long token addresses.
IDIVA.Pool memory _pool = IDIVA(_diva).getPoolParameters(_poolId);
​
// Early check that the pool's collateral token is associated with a registered collateral token.
// This ensures an immediate and graceful revert.
if (_wTokenToCollateralToken[_pool.collateralToken] == address(0)) {
revert CollateralTokenNotRegistered();
}
​
IERC20Metadata _shortTokenContract = IERC20Metadata(_pool.shortToken);
IERC20Metadata _longTokenContract = IERC20Metadata(_pool.longToken);
IERC20Metadata _collateralTokenContract = IERC20Metadata(_pool.collateralToken);
​
...
​
//@q what if _positionTokenAmount != type(uint256).max, does this revert if the amount is less than whats in the short or long contract
//@q why transfer when removing liquidity
@> _shortTokenContract.transferFrom(msg.sender /** from */, address(this) /** to */, _positionTokenAmountToRemove);
@> _longTokenContract.transferFrom(msg.sender /** from */, address(this) /** to */, _positionTokenAmountToRemove);
​
...
return _amountReturned;
}
​

Impact

This is problematic because the long and short tokens may belong to different addresses (if _addLiquidity assigned them to different recipients). As a result:

Denial of Service (DoS) Risk:

  • If the long and short tokens are held by different addresses, neither address can successfully call _removeLiquidity to retrieve collateral.

  • This effectively locks the liquidity in the protocol and prevents proper withdrawal.

Inconsistent User Experience:

  • Users who originally received separate long and short tokens may not expect that withdrawal requires both tokens to be held by the same wallet.

  • This creates unnecessary complexity and limits usability.

Tools Used

Manual review

Recommendations

Modify _removeLiquidity to allow separate addresses for long and short token withdrawals. Instead of enforcing msg.sender to hold both, allow:

  • _longOwner and _shortOwner parameters so that long and short tokens can be retrieved from different wallets.

  • Checking balances for each separately before executing transfers.

This way, any address that holds a valid amount of long or short tokens can participate in liquidity removal, ensuring fairer access and preventing DoS risks.

Updates

Lead Judging Commences

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[Invalid] Inconsistent recipient addresses

The `addLiquidity` allows the short and long recipients to be different addresses. Then if a given user has only one of the position tokens, he calls `redeemPositionToken` to redeem position token amount, if this user has amount of the both token types, he can call `removeLiquidity` and in that way an equal amount of short and long tokens will be burned.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.