Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Reentrancy Risk in `VaultRouterBranch::deposit` and `VaultRouterBranch::redeem` Functions

trepid

Description:
Both deposit and redeem functions in VaultRouterBranch.sol interact with external IERC4626 token contracts (indexToken). Specifically, they call IERC4626(indexTokenCache).deposit(...) in deposit and IERC4626(indexToken).redeem(...) in redeem.

impact:
These calls transfer tokens into and out of the VaultRouterBranch contract, respectively. If the indexToken contract is a malicious or vulnerable contract, it could potentially perform a reentrant call back into the VaultRouterBranch contract during these deposit or redeem operations.

Proof of Concept:

In deposit:

  1. User calls deposit in VaultRouterBranch.sol.

  2. VaultRouterBranch.sol transfers deposit assets from user to VaultRouterBranch.sol using safeTransferFrom.

  3. VaultRouterBranch.sol approves indexToken contract to spend assetsMinusFees.

  4. VaultRouterBranch.sol calls IERC4626(indexTokenCache).deposit(ctx.assetsMinusFees, msg.sender);

  5. Reentrancy Point: If the indexToken contract's deposit function (or any function it calls internally during deposit) performs a callback to VaultRouterBranch.sol (e.g., by calling a function that updates vault state or initiates another deposit/redeem operation), reentrancy can occur

Reentrancy Risk in deposit
function deposit(
uint128 vaultId,
uint128 assets,
uint128 minShares,
bytes memory referralCode,
bool isCustomReferralCode
)
external
{
if (assets == 0) revert Errors.ZeroInput("assets");
​
// load the mm engine configuration from storage
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
​
// enforce whitelist if enabled
address whitelistCache = marketMakingEngineConfiguration.whitelist;
if (whitelistCache != address(0)) {
if (!Whitelist(whitelistCache).verifyIfUserIsAllowed(msg.sender)) {
revert Errors.UserIsNotAllowed(msg.sender);
}
}
​
// fetch storage slot for vault by id, vault must exist with valid collateral
Vault.Data storage vault = Vault.loadLive(vaultId);
if (!vault.collateral.isEnabled) revert Errors.VaultDoesNotExist(vaultId);
​
// define context struct and get vault collateral asset
DepositContext memory ctx;
ctx.vaultAsset = vault.collateral.asset;
​
// prepare the `Vault::recalculateVaultsCreditCapacity` call
uint256[] memory vaultsIds = new uint256[](1);
vaultsIds[0] = uint256(vaultId);
​
// recalculates the vault's credit capacity
// note: we need to update the vaults credit capacity before depositing new assets in order to calculate the
// correct conversion rate between assets and shares, and to validate the involved invariants accurately
Vault.recalculateVaultsCreditCapacity(vaultsIds);
​
// load the referral module contract
ctx.referralModule = IReferral(marketMakingEngineConfiguration.referralModule);
​
// register the given referral code
if (referralCode.length != 0) {
ctx.referralModule.registerReferral(
abi.encode(msg.sender), msg.sender, referralCode, isCustomReferralCode
);
}
​
// cache the vault assets decimals value for gas savings
ctx.vaultAssetDecimals = vault.collateral.decimals;
​
// uint256 -> ud60x18 18 decimals
ctx.assetsX18 = Math.convertTokenAmountToUd60x18(ctx.vaultAssetDecimals, assets);
​
// cache the deposit fee
ctx.vaultDepositFee = ud60x18(vault.depositFee);
​
// if deposit fee is zero, skip needless processing
if (ctx.vaultDepositFee.isZero()) {
ctx.assetsMinusFees = assets;
} else {
// otherwise calculate the deposit fee
ctx.assetFeesX18 = ctx.assetsX18.mul(ctx.vaultDepositFee);
​
// ud60x18 -> uint256 asset decimals
ctx.assetFees = Math.convertUd60x18ToTokenAmount(ctx.vaultAssetDecimals, ctx.assetFeesX18);
​
// invariant: if vault enforces fees then calculated fee must be non-zero
if (ctx.assetFees == 0) revert Errors.ZeroFeeNotAllowed();
​
// enforce positive amount left over after deducting fees
ctx.assetsMinusFees = assets - ctx.assetFees;
if (ctx.assetsMinusFees == 0) revert Errors.DepositTooSmall();
}
​
// transfer tokens being deposited minus fees into this contract
IERC20(ctx.vaultAsset).safeTransferFrom(msg.sender, address(this), ctx.assetsMinusFees);
​
// transfer fees from depositor to fee recipient address
if (ctx.assetFees > 0) {
IERC20(ctx.vaultAsset).safeTransferFrom(
msg.sender, marketMakingEngineConfiguration.vaultDepositAndRedeemFeeRecipient, ctx.assetFees
);
}
​
// increase vault allowance to transfer tokens minus fees from this contract to vault
address indexTokenCache = vault.indexToken;
IERC20(ctx.vaultAsset).approve(indexTokenCache, ctx.assetsMinusFees);
​
// then perform the actual deposit
// NOTE: the following call will update the total assets deposited in the vault
// NOTE: the following call will validate the vault's deposit cap
// invariant: no tokens should remain stuck in this contract
ctx.shares = IERC4626(indexTokenCache).deposit(ctx.assetsMinusFees, msg.sender);
​
// assert min shares minted
if (ctx.shares < minShares) revert Errors.SlippageCheckFailed(minShares, ctx.shares);
​
// invariant: received shares must be > 0 even when minShares = 0; no donation allowed
if (ctx.shares == 0) revert Errors.DepositMustReceiveShares();
​
// emit an event
emit LogDeposit(vaultId, msg.sender, ctx.assetsMinusFees);
}

In redeem:

  1. User calls redeem in VaultRouterBranch.sol.

  2. VaultRouterBranch.sol calculates redeem amount and redeem fee.

  3. VaultRouterBranch.sol calls IERC4626(indexToken).redeem(ctx.sharesMinusRedeemFeesX18.intoUint256(), msg.sender, address(this)).

  4. Reentrancy Point: If the indexToken contract's redeem function (or any function it calls internally during redeem) performs a callback to VaultRouterBranch.sol, reentrancy can occur.

  5. VaultRouterBranch.sol then further calls IERC4626(indexToken).redeem(...) again to redeem fees. This second redeem call within the same redeem function significantly increases the reentrancy risk, as a malicious indexToken can potentially re-enter and exploit state inconsistencies between the two redeem calls or before/after either of them.

Reentrancy Risk in redeem
function redeem(uint128 vaultId, uint128 withdrawalRequestId, uint256 minAssets) external {
// fetch storage slot for vault by id
Vault.Data storage vault = Vault.loadLive(vaultId);
​
// load storage slot for previously created withdrawal request
WithdrawalRequest.Data storage withdrawalRequest =
WithdrawalRequest.loadExisting(vaultId, msg.sender, withdrawalRequestId);
​
// revert if withdrawal request already fulfilled
if (withdrawalRequest.fulfilled) revert Errors.WithdrawalRequestAlreadyFulfilled();
​
// revert if withdrawal request delay not yet passed
if (withdrawalRequest.timestamp + vault.withdrawalDelay > block.timestamp) {
revert Errors.WithdrawDelayNotPassed();
}
​
// prepare the `Vault::recalculateVaultsCreditCapacity` call
uint256[] memory vaultsIds = new uint256[](1);
vaultsIds[0] = uint256(vaultId);
​
// updates the vault's credit capacity before redeeming
Vault.recalculateVaultsCreditCapacity(vaultsIds);
​
// define context struct, get withdraw shares and associated assets
RedeemContext memory ctx;
ctx.shares = withdrawalRequest.shares;
ctx.expectedAssetsX18 = getIndexTokenSwapRate(vaultId, ctx.shares, false);
​
// load the mm engine configuration from storage
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
​
// cache vault's redeem fee
ctx.redeemFee = vault.redeemFee;
​
// get assets minus redeem fee
ctx.expectedAssetsMinusRedeemFeeX18 =
ctx.expectedAssetsX18.sub(ctx.expectedAssetsX18.mul(ud60x18(ctx.redeemFee)));
​
// calculate assets minus redeem fee as shares
ctx.sharesMinusRedeemFeesX18 =
getVaultAssetSwapRate(vaultId, ctx.expectedAssetsMinusRedeemFeeX18.intoUint256(), false);
​
// get the shares to send to the vault deposit and redeem fee recipient
ctx.sharesFees = ctx.shares - ctx.sharesMinusRedeemFeesX18.intoUint256();
​
// cache the vault's credit capacity before redeeming
ctx.creditCapacityBeforeRedeemUsdX18 = vault.getTotalCreditCapacityUsd();
​
// cache the locked credit capacity before redeeming
ctx.lockedCreditCapacityBeforeRedeemUsdX18 = vault.getLockedCreditCapacityUsd();
​
// redeem shares previously transferred to the contract at `initiateWithdrawal` and store the returned assets
address indexToken = vault.indexToken;
uint256 assets =
IERC4626(indexToken).redeem(ctx.sharesMinusRedeemFeesX18.intoUint256(), msg.sender, address(this));
​
// get the redeem fee
if (ctx.sharesFees > 0) {
IERC4626(indexToken).redeem(
ctx.sharesFees, marketMakingEngineConfiguration.vaultDepositAndRedeemFeeRecipient, address(this)
);
}
​
// require at least min assets amount returned
if (assets < minAssets) revert Errors.SlippageCheckFailed(minAssets, assets);
​
// invariant: received assets must be > 0 even when minAssets = 0
if (assets == 0) revert Errors.RedeemMustReceiveAssets();
​
// if the credit capacity delta is greater than the locked credit capacity before the state transition, revert
if (
ctx.creditCapacityBeforeRedeemUsdX18.sub(vault.getTotalCreditCapacityUsd()).lte(
ctx.lockedCreditCapacityBeforeRedeemUsdX18.intoSD59x18()
)
) {
revert Errors.NotEnoughUnlockedCreditCapacity();
}
​
// set withdrawal request to fulfilled
withdrawalRequest.fulfilled = true;
​
// emit an event
emit LogRedeem(vaultId, msg.sender, ctx.sharesMinusRedeemFeesX18.intoUint256());
}

Recomended Mitigation:
Implement Reentrancy Guard: implement OpenZeppelin's ReentrancyGuard modifier on both deposit and redeem functions in VaultRouterBranch.sol

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!