Submission Details
Severity:
Users can deposit small amounts of tokens but they can't remove them later
Summary
Users are able to deposit/donate/add liquidity with any small amount of tokens they want, but for some small amounts they are not able to withdraw them later.
Vulnerability Details
Users can deposit tokens to a pool either at its creation through createContingentPool(...) or in an existing pool through addLiquidity(...). Users can deposit any amount of tokens they wish since there is no amount check. When users call removeLiquidity(...) to withdraw tokens, the DIVA contract corresponding function is called:
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);
.
.
// 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;
.
.
return _amountReturned;
}
This function at the DIVA protocol calculates a protocol fee. If this protocol fee is 0, the transaction reverts:
function _removeLiquidityLib(
RemoveLiquidityParams memory _removeLiquidityParams,
LibDIVAStorage.Pool storage _pool
) internal returns (uint256 collateralAmountRemovedNet) {
// Get reference to relevant storage slot
LibDIVAStorage.GovernanceStorage storage gs = LibDIVAStorage
._governanceStorage();
.
.
uint256 _protocolFee;
uint256 _settlementFee;
if (_fees.protocolFee > 0) {
// Calculate protocol fees to charge (note that collateral amount
// to return is equal to `_amount`)
_protocolFee = _calcFee(
_fees.protocolFee,
_removeLiquidityParams.amount,
IERC20Metadata(_pool.collateralToken).decimals()
);
// User has to increase `_amount` if fee is 0
@> if (_protocolFee == 0) revert ZeroProtocolFee();
} // else _protocolFee = 0 (default value for uint256)
if (_fees.settlementFee > 0) {
// Calculate settlement fees to charge
_settlementFee = _calcFee(
_fees.settlementFee,
_removeLiquidityParams.amount,
IERC20Metadata(_pool.collateralToken).decimals()
);
// User has to increase `_amount` if fee is 0
@> if (_settlementFee == 0) revert ZeroSettlementFee();
} // else _settlementFee = 0 (default value for uint256)
.
}
For small amounts of tokens it's possible for this fee to round down to 0:
/**
* @notice Function to calculate the fee amount for a given collateral amount.
* @dev Output is an integer expressed with collateral token decimals.
* As fee parameter has 18 decimals but collateral tokens may have
* less, scaling needs to be applied when using `SafeDecimalMath` library.
* @param _fee Percentage fee expressed as an integer with 18 decimals
* (e.g., 0.25% is 2500000000000000).
* @param _collateralAmount Collateral amount that is used as the basis for
* the fee calculation expressed as an integer with collateral token decimals.
* @param _collateralTokenDecimals Collateral token decimals.
* @return The fee amount expressed as an integer with collateral token decimals.
*/
function _calcFee(
uint96 _fee,
uint256 _collateralAmount,
uint8 _collateralTokenDecimals
) internal pure returns (uint256) {
uint256 _SCALINGFACTOR;
unchecked {
// Cannot over-/underflow as collateral token decimals are restricted to
// a minimum of 6 and a maximum of 18.
_SCALINGFACTOR = uint256(10**(18 - _collateralTokenDecimals));
}
uint256 _feeAmount = uint256(_fee).multiplyDecimal(
_collateralAmount * _SCALINGFACTOR
) / _SCALINGFACTOR;
return _feeAmount;
}
function multiplyDecimal(uint256 x, uint256 y)
internal
pure
returns (uint256)
{
// Divide by UNIT to remove the extra factor introduced by the product
return (x * y) / UNIT;
}
According to the natspec, a 0.25% fee is passed as 2500000000000000 so we'll use that _fee with a 6 decimal token like USDT and a _collateralAmount = 100:
UNIT = 10**18 // According to the SafeDecimalMath.sol contract
_SCALINGFACTOR = uint256(10**(18 - _collateralTokenDecimals));
_SCALINGFACTOR = 10**18-6 = 10**12
uint256 _feeAmount = uint256(_fee).multiplyDecimal(
_collateralAmount * _SCALINGFACTOR
) / _SCALINGFACTOR;
uint256 _feeAmount = 2500000000000000.multiplyDecimal(
100 * 10**12
) / 10**12;
uint256 _feeAmount = 2500000000000000 * ((100 * 10**12) / 10**18)
) / 10**12;
uint256 _feeAmount = 0.25 // Which rounds down to 0
Impact
Users have no restriction to deposit small amounts but they are unwithdrawable later, resulting in loss of funds.
POC
First, create a new test file in the test folder (e.g. Issues.test.ts). Then, paste the following code:
import { expect } from "chai";
import hre, { ethers } from "hardhat";
const { parseUnits, toBeHex } = ethers;
// import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers";
import { mine, loadFixture } from "@nomicfoundation/hardhat-network-helpers";
import {
AaveDIVAWrapper,
IAave,
IDIVA,
MockERC20,
ERC20,
WToken,
} from "../typechain-types";
import {
SetupOutput,
CreateContingentPoolParams,
AddLiquidityParams,
RemoveLiquidityParams,
SetupWithPoolResult,
SetupWithConfirmedPoolResult,
} from "../constants/types";
import { DIVA_ADDRESS, AAVE_ADDRESS } from "../utils/addresses";
import { getExpiryTime, getLastTimestamp } from "../utils/blocktime";
import {
getPoolIdFromAaveDIVAWrapperEvent,
getPoolIdFromDIVAEvent,
} from "../utils/eventUtils";
import { calcTotalDIVAFee } from "../utils/diva";
import { NETWORK_CONFIGS } from "../utils/addresses";
import { NETWORK } from "../hardhat.config";
// Get network hardhat.config.ts
const network = NETWORK;
const networkConfig = NETWORK_CONFIGS[network];
// Configure test collateral token and holder account to impersonate.
// Note: The holder account must have sufficient balance of the collateral token.
// IMPORTANT: The token key (e.g. 'USDT') must be the same for both collateralToken and collateralTokenHolder
// to ensure they match.
const collateralToken = networkConfig.collateralTokens.USDT.address;
const collateralTokenHolder = networkConfig.collateralTokens.USDT.holder;
const collateralTokenUnsupported = networkConfig.unsupportedToken.address;
// Second collateral token used for testing batch registration functionality.
// IMPORTANT: Must differ from first token to avoid duplicate registration errors.
const collateralToken2 = networkConfig.collateralTokens.USDC.address;
const divaAddress = DIVA_ADDRESS[network];
const aaveAddress = AAVE_ADDRESS[network];
describe("AaveDIVAWrapper Issues Test", function () {
// Test setup function
async function setup(): Promise<SetupOutput> {
// Get the Signers
const [owner, acc2, acc3, dataProvider] = await ethers.getSigners();
// Impersonate account
await hre.network.provider.request({
method: "hardhat_impersonateAccount",
params: [collateralTokenHolder],
});
const impersonatedSigner = await ethers.getSigner(collateralTokenHolder);
// Now `impersonatedSigner` can be used to send transactions from the impersonated account
// Create a new contract instance to interact with the collateral token
const collateralTokenContract: ERC20 = await ethers.getContractAt(
"ERC20",
collateralToken,
);
// Get the decimals of the collateral token
const collateralTokenDecimals = Number(
await collateralTokenContract.decimals(),
);
// Confirm that the balance is greater than 1000.
const balance = await collateralTokenContract.balanceOf(
impersonatedSigner.address,
);
expect(balance).to.be.gt(parseUnits("1000", collateralTokenDecimals));
// Generate a dummy token and send it to owner
const dummyTokenDecimals = 18;
const dummyTokenContract: MockERC20 = await ethers.deployContract(
"MockERC20",
[
"DummyToken", // name
"DT", // symbol
ethers.parseUnits("10000", dummyTokenDecimals), // totalSupply
owner.address, // recipient
dummyTokenDecimals, // decimals
0, // feePct
],
);
await dummyTokenContract.waitForDeployment();
// Deploy AaveDIVAWrapper contract
const aaveDIVAWrapper: AaveDIVAWrapper = await ethers.deployContract(
"AaveDIVAWrapper",
[divaAddress, aaveAddress, owner.address],
);
await aaveDIVAWrapper.waitForDeployment();
// Connect to DIVA Voucher contract instance
const diva: IDIVA = await ethers.getContractAt("IDIVA", divaAddress);
const aave: IAave = await ethers.getContractAt("IAave", aaveAddress);
// Approve AaveDIVAWrapper contract with impersonatedSigner
await collateralTokenContract
.connect(impersonatedSigner)
.approve(aaveDIVAWrapper.target, ethers.MaxUint256);
// Approve DIVA contract with impersonatedSigner
await collateralTokenContract
.connect(impersonatedSigner)
.approve(diva.target, ethers.MaxUint256);
// Default create contingent pool parameters. Can be inherited via the spread operator
// inside the tests and overridden as needed.
const createContingentPoolParams: CreateContingentPoolParams = {
referenceAsset: "BTC/USD",
expiryTime: await getExpiryTime(60 * 60 * 2),
floor: parseUnits("100"),
inflection: parseUnits("150"),
cap: parseUnits("200"),
gradient: parseUnits("0.5", collateralTokenDecimals),
collateralAmount: 100,
//collateralAmount: parseUnits("100", collateralTokenDecimals),
collateralToken: collateralToken,
dataProvider: dataProvider.address,
capacity: ethers.MaxUint256,
longRecipient: impersonatedSigner.address,
shortRecipient: impersonatedSigner.address,
permissionedERC721Token: ethers.ZeroAddress,
};
return {
dummyTokenContract,
dummyTokenDecimals,
owner,
acc2,
acc3,
dataProvider,
impersonatedSigner,
collateralTokenContract,
collateralTokenDecimals,
aaveDIVAWrapper,
aave,
diva,
createContingentPoolParams,
};
}
async function setupWithPool(): Promise<SetupWithPoolResult> {
// Fetch setup fixture.
const s: SetupOutput = await loadFixture(setup);
// Register the collateral token and connect to wToken contract.
await s.aaveDIVAWrapper
.connect(s.owner)
.registerCollateralToken(collateralToken);
const wTokenAddress: string =
await s.aaveDIVAWrapper.getWToken(collateralToken);
const wTokenContract: WToken = await ethers.getContractAt(
"WToken",
wTokenAddress,
);
// Connect to the aToken contract associated with the collateral token.
const aTokenAddress: string =
await s.aaveDIVAWrapper.getAToken(collateralToken);
const aTokenContract: ERC20 = await ethers.getContractAt(
"IERC20",
aTokenAddress,
);
// Fund impersonatedSigner with native token (e.g., MATIC on Polygon) to be able to pay for gas.
await hre.network.provider.send("hardhat_setBalance", [
s.impersonatedSigner.address,
toBeHex(parseUnits("10", 18)), // Sending 10 native tokens
]);
// Create a new contingent pool via the AaveDIVAWrapper contract.
await s.aaveDIVAWrapper
.connect(s.impersonatedSigner)
.createContingentPool(s.createContingentPoolParams);
// Fetch the poolId from the event and fetch pool parameters from DIVA Protocol.
const poolId: string = await getPoolIdFromAaveDIVAWrapperEvent(
s.aaveDIVAWrapper,
);
const poolParams: IDIVA.PoolStructOutput =
await s.diva.getPoolParameters(poolId);
// Connect to the short and long token contracts.
const shortTokenContract: ERC20 = await ethers.getContractAt(
"ERC20",
poolParams.shortToken,
);
const longTokenContract: ERC20 = await ethers.getContractAt(
"ERC20",
poolParams.longToken,
);
// Approve the AaveDIVAWrapper contract to transfer the short and long tokens.
await shortTokenContract
.connect(s.impersonatedSigner)
.approve(s.aaveDIVAWrapper.target, ethers.MaxUint256);
await longTokenContract
.connect(s.impersonatedSigner)
.approve(s.aaveDIVAWrapper.target, ethers.MaxUint256);
// Default parameters for removeLiquidity function.
const r: RemoveLiquidityParams = {
poolId: poolId,
positionTokenAmount: 99,
//positionTokenAmount: parseUnits("10", s.collateralTokenDecimals),
recipient: s.impersonatedSigner.address,
};
// Default parameters for addLiquidity function.
const a: AddLiquidityParams = {
poolId: poolId,
collateralAmount: parseUnits("10", s.collateralTokenDecimals),
longRecipient: s.impersonatedSigner.address,
shortRecipient: s.impersonatedSigner.address,
};
// Calculate DIVA fee (claimable after the pool has been confirmed inside DIVA Protocol).
const divaFees = await calcTotalDIVAFee(
s.diva,
poolParams,
BigInt(r.positionTokenAmount),
s.collateralTokenDecimals,
);
// Make some assertion to ensure that the setup satisfies required conditions.
expect(r.positionTokenAmount).to.be.lt(
s.createContingentPoolParams.collateralAmount,
);
//expect(divaFees).to.gt(0);
return {
s,
wTokenContract,
wTokenAddress,
aTokenContract,
aTokenAddress,
poolId,
poolParams,
shortTokenContract,
longTokenContract,
r,
divaFees,
a,
};
}
describe("removeLiquidity", async () => {
let s: SetupOutput;
let wTokenContract: WToken;
let aTokenContract: ERC20;
let shortTokenContract: ERC20;
let longTokenContract: ERC20;
let r: RemoveLiquidityParams;
let divaFees: bigint;
beforeEach(async () => {
({
s,
wTokenContract,
aTokenContract,
shortTokenContract,
longTokenContract,
r,
divaFees,
} = await setupWithPool());
expect(r.positionTokenAmount).to.be.gt(0);
expect(r.positionTokenAmount).to.be.lt(
s.createContingentPoolParams.collateralAmount,
);
//expect(divaFees).to.gt(0);
});
it.only("Should revert", async () => {
await s.aaveDIVAWrapper
.connect(s.impersonatedSigner)
.removeLiquidity(r.poolId, r.positionTokenAmount, r.recipient);
});
});
});
The code used is very similar to the already existing test file, but with other amounts of depositing and removing. You can see the changes made at lines 128 and 217. Deposits 100 tokens and tries to withdraw 99.
Notice the test reverts with reverted with an unrecognized custom error (return data: 0x9c450e72). If we take the ZeroSettlementFee() error and get its selector, it is 0x9c450e72.
Tools Used
Manual review
Recommendations
Create a minimum of how many tokens a user can deposit.
Prize pool breakdown
Total prize
15,000 USDC
nSLOC
51929 USDC / LOC
14,000
1,000
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.