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 11 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.

Give us feedback!