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

Incorrect Handling of Maximum Token Amount in `AaveDIVAWrapperCore::_removeLiquidity` Leading to Zero Protocol Fee Reversion

lhoussaineph2001

Summary

A critical issue has been identified in AaveDIVAWrapperCore::_removeLiquidity function of the AaveDIVAWrapperCore contract when the liquidity provider (LP) is not the recipient of both short and long position tokens. This bug results in the inability to remove liquidity when the LP inputs `type(uint256).max` to withdraw all collateral, leading to a transaction revert due to a zero protocol fee calculation.

Vulnerability Details

When an LP adds liquidity but assigns different addresses for shortRecipient and longRecipient, they do not hold the position tokens themselves. Consequently, when attempting to remove liquidity, the LP's balance for both short and long tokens is zero. This results in the following:

​
uint256 _userBalanceShort = _shortTokenContract.balanceOf(msg.sender); // retunr 0 if msg.sender != shortRecipient
uint256 _userBalanceLong = _longTokenContract.balanceOf(msg.sender); // return 0 if msg.sender != longRecipient
​

If `_positionTokenAmount == type(uint256).max`

​
//@audit _positionTokenAmountToRemove = 0 return the min balance witch is zero
if (_positionTokenAmount == type(uint256).max) {
_positionTokenAmountToRemove = _userBalanceShort > _userBalanceLong ? _userBalanceLong : _userBalanceShort;
}
​

The `removeLiquidity` function is called with an amount of zero:

​
IDIVA(_diva).removeLiquidity(_poolId, 0);
​
​
​

This results in a revert due to the zero protocol fee error in the DIVA Protocol's LibDIVA contract.

Smart Contract References

https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/facets/LiquidityFacet.sol#L54
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/facets/LiquidityFacet.sol#L77
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L877
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L918
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L921
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L52
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L420
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L407
https://github.com/divaprotocol/diva-protocol-v1/blob/1263828cd5c5b2192e876edef90444448e66176d/contracts/libraries/LibDIVA.sol#L927

Impact

When a liquidity provider uses `type(uint256).max` to remove liquidity, the transaction fails due to zero balance checks on position tokens, resulting in locked collateral and inability to withdraw funds.

In `test/AaveDIVAWrapper.t.sol` :

​
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
​
import {IAaveDIVAWrapper} from "../src/interfaces/IAaveDIVAWrapper.sol";
import {IAave} from "../src/interfaces/IAave.sol";
import {IDIVA} from "../src/interfaces/IDIVA.sol";
​
import "../src/AaveDIVAWrapper.sol";
import "../src/interfaces/IDIVA.sol";
import "lib/openzeppelin-contracts/contracts/interfaces/IERC20.sol";
​
interface IUSDC {
function approve(address spender, uint256 amount) external;
function transfer(address recipient, uint256 amount) external;
function mint(address account, uint256 amount) external;
function balanceOf(address) external view returns (uint256);
function transferFrom(address sender, address recipient, uint256 amount) external;
function decimals() external view returns (uint8);
}
​
contract AaveDIVAWrapperTest is Test {
​
AaveDIVAWrapperCore wrapper;
​
address trader = address(0x4D8336bDa6C11BD2a805C291Ec719BaeDD10AcB9); // in polygon
address shortRecipient;
address longRecipient;
bytes32 poolId;
​
IAave _aavePool = IAave(0x794a61358D6845594F94dc1DB02A252b5b4814aD); // in polygon
IDIVA _diva = IDIVA(0x2C9c47E7d254e493f02acfB410864b9a86c28e1D); // in polygon
IUSDC USDC = IUSDC(0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359); // in polygon
​
​
​
function setUp() public {
​
// Deploy or connect to the necessary contracts
wrapper = new AaveDIVAWrapper(address(_diva),address(_aavePool),address(this));
​
shortRecipient = address(0x1); // short recipent address
longRecipient = address(0x2); // log recipent address
​
vm.startPrank(trader);
IUSDC(USDC).approve(address(this), type(uint256).max);
vm.stopPrank();
IUSDC(USDC).transferFrom(trader, address(this), 100* 10**USDC.decimals());
wrapper.registerCollateralToken(address(USDC));
​
}
​
​
​
​
​
modifier createPool {
​
// Create a pool
USDC.approve(address(wrapper), type(uint256).max);
​
​
poolId = wrapper.createContingentPool(IAaveDIVAWrapper.PoolParams({
referenceAsset: "BTC/USD",
expiryTime: uint96(block.timestamp + 86400),
floor: 100,
inflection: 150,
cap: 200,
gradient: uint256(5 * 10 ** (USDC.decimals()-1)),
collateralAmount: 100 * 10**USDC.decimals(),
collateralToken: address(USDC),
dataProvider: address(0x11),
capacity: type(uint256).max,
longRecipient: longRecipient,
shortRecipient: shortRecipient,
permissionedERC721Token: address(0)
}));
​
_;
}
​
​
modifier addliquidity {
​
vm.startPrank(trader);
USDC.approve(address(wrapper), type(uint256).max);
wrapper.addLiquidity(poolId,100 *10**USDC.decimals() , longRecipient, shortRecipient);
vm.stopPrank();
_;
}
​
function testRemoveLiquidity() external createPool addliquidity{
​
vm.startPrank(trader);
​
/**
* $ cast keccak256 "ZeroProtocolFee()"
0x9c450e723e108f09f1d53643b4e83d348b7c37e10965f616462e3f3d8abb4d23
*/
​
// Expect revert with the specific selector for "ZeroProtocolFee()"
vm.expectRevert(bytes4(keccak256("ZeroProtocolFee()")));
wrapper.removeLiquidity(poolId, type(uint256).max , address(this));
​
vm.stopPrank();
}
}
​

Run this command :

$ forge test --mt testRemoveLiquidity --fork-url https://polygon-mainnet.g.alchemy.com/v2/Yn7tIZfhjHHmJQbJ4SeXkpuRju0udqkR -vvv
​

Test pass:

​
[⠒] Compiling...
[⠆] Compiling 1 files with Solc 0.8.26
[⠰] Solc 0.8.26 finished in 2.13s
Compiler run successful!
​
Ran 1 test for test/AaveDivaWrapper.t.sol:AaveDIVAWrapperTest
[PASS] testRemoveLiquidity() (gas: 1288122)
​
​

Tools Used

  • Manuial Review

  • Foundry Framework

Recommendations

​
- uint256 _userBalanceShort = _shortTokenContract.balanceOf(msg.sender);
- uint256 _userBalanceLong = _longTokenContract.balanceOf(msg.sender);
​
+ uint256 _userBalanceShort = _shortTokenContract.balanceOf(_pool.shortRecipient);
+ uint256 _userBalanceLong = _longTokenContract.balanceOf(_pool.longRecipient);
​
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.