HardhatDeFi
15,000 USDC
View results
Submission Details
Severity: low
Invalid

Inconsistent Recipient Address Handling in addLiquidity and Related Functions: AaveDIVAWrapper.sol

Summary

The addLiquidity function accepts two recipient addresses: _longRecipient and _shortRecipient. However, this dual-address logic is not carried forward in subsequent functions, such as removeLiquidity, redeemPositionToken, and redeemWToken, which instead use a single address _recipient.

This inconsistency can lead to potential tracking issues, as the contract does not maintain the relationship between _longRecipient and _shortRecipient when handling liquidity.

Vulnerability Details

The mismatch between the parameters in addLiquidity and subsequent functions creates a risk of inconsistencies in tracking recipient addresses. Below is an example from some of the functions illustrating the issue:

function addLiquidity(
bytes32 _poolId,
uint256 _collateralAmount,
address _longRecipient, // Dual recipient addresses
address _shortRecipient // Dual recipient addresses
) external override nonReentrant {
_addLiquidity(_poolId, _collateralAmount, _longRecipient, _shortRecipient);
}
function removeLiquidity(
bytes32 _poolId,
uint256 _positionTokenAmount,
address _recipient // Single recipient address
) external override nonReentrant returns (uint256) {
return _removeLiquidity(_poolId, _positionTokenAmount, _recipient);
}
function redeemWToken(
address _wToken,
uint256 _wTokenAmount,
address _recipient // Single recipient address
) external override nonReentrant returns (uint256) {
return _redeemWToken(_wToken, _wTokenAmount, _recipient);
}
function batchAddLiquidity(AddLiquidityArgs[] calldata _addLiquidityArgs) external override nonReentrant {
uint256 _length = _addLiquidityArgs.length;
for (uint256 i = 0; i < _length; i++) {
_addLiquidity(
_addLiquidityArgs[i].poolId,
_addLiquidityArgs[i].collateralAmount,
_addLiquidityArgs[i].longRecipient, // Dual recipient addresses
_addLiquidityArgs[i].shortRecipient // Dual recipient addresses
);
}
}

Impact

Loss of Tracking:

  • If a user calls addLiquidity and passes both _longRecipient and _shortRecipient, the contract does not maintain the distinction between the two addresses for later use.

  • Functions like removeLiquidity or redeemWToken relay on a single _recipient address, potentially leading to:

    • Incorrect redemption or removal of liquidity.

    • Locked funds for one of the recipients.

Tools Used

Manual review

Recommendations

1 Simplify addLiquidity:

Modify addLiquidity to accept only a single recipient address (_recipient), ensuring consistency across all functions.

  • Example:

    function addLiquidity(
    bytes32 _poolId,
    uint256 _collateralAmount,
    address _recipient
    ) external override nonReentrant {
    _addLiquidity(_poolId, _collateralAmount, _recipient, _recipient);
    }

2 Update Other Functions:

Modify functions such as removeLiquidity , redeemWToken and rest of the functions to handle both _longRecipient and _shortRecipient. This approach ensures that the dual-address logic from addLiquidity is preserved throughout the contract.

  • Example:

    function removeLiquidity(
    bytes32 _poolId,
    uint256 _positionTokenAmount,
    address _longRecipient,
    address _shortRecipient
    ) external override nonReentrant returns (uint256) {
    // Logic to handle both recipients
    }

The mismatch between addLiquidity and subsequent functions can lead to significant tracking and usability issues. Adopting one of the above recommendations will ensure consistency and prevent the loss of recipient address data.

Updates

Lead Judging Commences

bube Lead Judge 5 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.