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

-- `Liquidity Providers` might be unable to remove liquidity via AaveDIVAWrapper::removeLiquidity()if `longRecipient` and/or `shortRecipient` addresses are different from `msg.sender` in AaveDIVAWrapper::addLiquidity()

ikpongjos

Summary

Vulnerability Details

Liquidity Providers will interact with AaveDIVAWrapper::addLiquidity() which will call AaveDIVAWrapperCore::_addLiquidity() which does receive 2 arbitrary addresses.

#AaveDIVAWrapperCore::_addLiquidity()
# https://github.com/Cyfrin/2025-01-diva/blob/1b6543768c341c2334cdff87b6dd627ee2f62c89/contracts/src/AaveDIVAWrapperCore.sol#L167-L193
​
function _addLiquidity(
bytes32 _poolId,
uint256 _collateralAmount,
address _longRecipient, <@
address _shortRecipient <@
) internal {
​
#SNIPPED
​
}

Problem is that these addresses can be different from the msg.sender adding the liquidity.
This is seen in the AaveDIVAWrapperCore::_removeLiquidity() that checks for the shortToken and longToken balances of the msg.sender, the function caller. This address, though actually added the liquidity, might be different from the rightful holders of the shortToken and longToken and as such won't be able to call AaveDIVAWrapper::removeLiquidity() / AaveDIVAWrapperCore::_removeLiquidity() with success.

#AaveDIVAWrapperCore::_removeLiquidity()
#https://github.com/Cyfrin/2025-01-diva/blob/1b6543768c341c2334cdff87b6dd627ee2f62c89/contracts/src/AaveDIVAWrapperCore.sol#L198-L251
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);
​
// Use the user's min short/long token balance if `_positionTokenAmount` equals `type(uint256).max`.
// That corresponds to the maximum amount that the user can remove from the pool.
uint256 _userBalanceShort = _shortTokenContract.balanceOf(msg.sender); <@
uint256 _userBalanceLong = _longTokenContract.balanceOf(msg.sender); <@
uint256 _positionTokenAmountToRemove = _positionTokenAmount;
if (_positionTokenAmount == type(uint256).max) {
_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 /** from */, address(this) /** to */, _positionTokenAmountToRemove);
_longTokenContract.transferFrom(msg.sender /** from */, address(this) /** to */, _positionTokenAmountToRemove);
​
// Remove liquidity on DIVA Protocol to receive wTokens, and calculate the returned wToken amount (net of DIVA fees)
// as DIVA Protocol's removeLiquidity function does not return the amount of collateral token received.
uint256 _wTokenBalanceBeforeRemoveLiquidity = _collateralTokenContract.balanceOf(address(this));
IDIVA(_diva).removeLiquidity(_poolId, _positionTokenAmountToRemove);
uint256 _wTokenAmountReturned = _collateralTokenContract.balanceOf(address(this)) -
_wTokenBalanceBeforeRemoveLiquidity;
​
// Conscious decision to omit an early zero amount check here as it will either revert inside `removeLiquidity` due to
// zero DIVA fees (if DIVA fee pct != 0) or in the subsequent call to Aave's `withdraw` function inside `_redeemWTokenPrivate`.
​
// Withdraw collateral token from Aave, burn wTokens owned by this contract and transfer collateral token to `_recipient`.
uint256 _amountReturned = _redeemWTokenPrivate(
_pool.collateralToken, // wToken
_wTokenAmountReturned,
_recipient,
address(this)
);
​
return _amountReturned;
}
​
​

Impact

Liquidity Providers might be unable to remove liquidity via AaveDIVAWrapper::removeLiquidity()if longRecipient and/or shortRecipient addresses are different from msg.sender in AaveDIVAWrapper::addLiquidity() / AaveDIVAWrapperCore::_addLiquidity().

Tools Used

Manual review.

Recommendations

  1. Have mapping that stores each liquidity provider to their provided short and long recipients

  2. Have liquidity providers provide the addresses of their short and long token recipients and check against the mapping that such correlation exists.

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.