Submission Details
Severity:
`initiateSwap` Does not check that the vault is Live will result in lose for user
Summary
when user wants to swap usdToken for vault assets , the user will initiate swap request than the keeper will call FulfillSwap function to complete the swap, However in first call we did not check for LiveVault which will got executed when vault is not live, in 2nd call to fulfill the request it will revert because here we check for liveVault.
Vulnerability Details
Let's have a look on the code , when user submit the request to initiate swap he will calls initiateSwap function:
/src/market-making/branches/StabilityBranch.sol:201
201: function initiateSwap(
202: uint128[] calldata vaultIds,
203: uint128[] calldata amountsIn,
204: uint128[] calldata minAmountsOut
205: )
206: external
207: {
...
216:
217: // working data
218: InitiateSwapContext memory ctx;
219:
220: // cache the vault's index token and asset addresses
221: // @audit why dot check isLive ?
222: Vault.Data storage currentVault = Vault.load(vaultIds[0]);
223: ctx.initialVaultIndexToken = currentVault.indexToken;
224: ctx.initialVaultCollateralAsset = currentVault.collateral.asset;
225:
226: // load collateral data; must be enabled
227: Collateral.Data storage collateral = Collateral.load(ctx.initialVaultCollateralAsset);
228: collateral.verifyIsEnabled();
229:
230: // load market making engine config
231: MarketMakingEngineConfiguration.Data storage configuration = MarketMakingEngineConfiguration.load();
232:
233: // load usd token swap data
234: UsdTokenSwapConfig.Data storage tokenSwapData = UsdTokenSwapConfig.load();
235:
236: // cache additional common fields
237: // ctx.collateralPriceX18 in zaros internal precision
238: ctx.collateralPriceX18 = currentVault.collateral.getPrice();
239: ctx.maxExecTime = uint120(tokenSwapData.maxExecutionTime);
240: // ctx.vaultAssetBalance in native precision of ctx.initialVaultCollateralAsset
241: ctx.vaultAssetBalance = IERC20(ctx.initialVaultCollateralAsset).balanceOf(ctx.initialVaultIndexToken);
242:
243: for (uint256 i; i < amountsIn.length; i++) {
244: // for all but first iteration, refresh the vault and enforce same collateral asset
245: if (i != 0) {
246: currentVault = Vault.load(vaultIds[i]);
247:
248: // revert for swaps using vaults with different collateral assets
249: if (currentVault.collateral.asset != ctx.initialVaultCollateralAsset) {
250: revert Errors.VaultsCollateralAssetsMismatch();
251: }
252:
253: // refresh current vault balance in native precision of ctx.initialVaultCollateralAsset
254: ctx.vaultAssetBalance = IERC20(ctx.initialVaultCollateralAsset).balanceOf(currentVault.indexToken);
255: }
From the above it can be seen that we did not check that the vault is love or not.
but in case of fulfill swap :
/src/market-making/branches/StabilityBranch.sol:330
330: function fulfillSwap(
331: address user,
332: uint128 requestId,
333: bytes calldata priceData,
334: address engine
335: )
336: external
337: onlyRegisteredSystemKeepers
338: {
339: // load request for user by id
340: UsdTokenSwapConfig.SwapRequest storage request = UsdTokenSwapConfig.load().swapRequests[user][requestId];
341:
342: // revert if already processed
343: if (request.processed) {
344: revert Errors.RequestAlreadyProcessed(user, requestId);
345: }
346:
347: // working data
348: FulfillSwapContext memory ctx;
349:
350: // if request dealine expired revert
351: ctx.deadline = request.deadline;
352: if (ctx.deadline < block.timestamp) {
353: revert Errors.SwapRequestExpired(user, requestId, ctx.deadline);
354: }
355:
356: // set request processed to true
357: request.processed = true;
358:
359: // load market making engine config
360: MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
361: MarketMakingEngineConfiguration.load();
362:
363: // load vault data
364: ctx.vaultId = request.vaultId;
365: Vault.Data storage vault = Vault.loadLive(ctx.vaultId);
366:
At line 365 we only fetch live vault So in case if vault is not live it will revert.
The user have no way to wait for deadline and call refundSwap function will charge baseFee from user, thus user will suffer a lose here.
##POC :
function test_vault_not_live(
)
external
whenCallerIsKeeper
whenRequestWasNotYetProcessed
whenSwapRequestNotExpired
{
uint256 vaultId = 100;
uint256 marketId = 100;
uint256 vaultAssetsBalance = 5000e16;
uint256 swapAmount = 1000e6;
int256 vaultDebtAbsUsd = int256(2e18);
bool useCredit= false;
TestFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsNotZero_Context memory ctx;
ctx.fuzzVaultConfig = getFuzzVaultConfig(vaultId);
ctx.oneAsset = 10 ** ctx.fuzzVaultConfig.decimals;
changePrank({ msgSender: users.owner.account });
marketMakingEngine.configureUsdTokenSwapConfig(1, 30, type(uint96).max);
Vault.UpdateParams memory params = Vault.UpdateParams({
vaultId: ctx.fuzzVaultConfig.vaultId,
depositCap: uint128(100000e18),
withdrawalDelay: uint128(230),
isLive: false,
lockedCreditRatio: 0
});
// it should emit update event
marketMakingEngine.updateVaultConfiguration(params);
changePrank({ msgSender: users.naruto.account });
deal({
token: address(ctx.fuzzVaultConfig.asset),
to: ctx.fuzzVaultConfig.indexToken,
give: vaultAssetsBalance
});
swapAmount = vaultAssetsBalance / 1e9;
deal({ token: address(usdToken), to: users.naruto.account, give: swapAmount });
ctx.fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
ctx.engine = IMockEngine(perpMarketsCreditConfig[ctx.fuzzPerpMarketCreditConfig.marketId].engine);
// we update the mock engine's unrealized debt in order to update the vault's total debt state
ctx.engine.setUnrealizedDebt(useCredit ? -int256(vaultDebtAbsUsd) : int256(vaultDebtAbsUsd));
ctx.minAmountOut = 0;
UD60x18 priceUsdX18 = IPriceAdapter(vaultsConfig[ctx.fuzzVaultConfig.vaultId].priceAdapter).getPrice();
ctx.priceData = getMockedSignedReport(ctx.fuzzVaultConfig.streamId, priceUsdX18.intoUint256());
ctx.usdTokenSwapKeeper = usdTokenSwapKeepers[ctx.fuzzVaultConfig.asset];
ctx.amountOut =
marketMakingEngine.getAmountOfAssetOut(ctx.fuzzVaultConfig.vaultId, ud60x18(swapAmount), priceUsdX18);
vm.assume(ctx.amountOut.intoUint256() > 0);
ctx.minAmountOut = ctx.amountOut.intoUint128();
initiateUsdSwap(uint128(ctx.fuzzVaultConfig.vaultId), swapAmount, ctx.minAmountOut);
(ctx.baseFeeX18, ctx.swapFeeX18) = marketMakingEngine.getFeesForAssetsAmountOut(ctx.amountOut, priceUsdX18);
ctx.amountOutAfterFee = convertUd60x18ToTokenAmount(
ctx.fuzzVaultConfig.asset, ctx.amountOut.sub(ctx.baseFeeX18.add(ctx.swapFeeX18))
);
changePrank({ msgSender: ctx.usdTokenSwapKeeper });
ctx.protocolSwapFee = ctx.swapFeeX18.mul(ud60x18(marketMakingEngine.exposed_getTotalFeeRecipientsShares()));
ctx.protocolReward =
convertUd60x18ToTokenAmount(ctx.fuzzVaultConfig.asset, ctx.baseFeeX18.add(ctx.protocolSwapFee));
ctx.requestId = 1;
marketMakingEngine.fulfillSwap(
users.naruto.account, ctx.requestId, ctx.priceData, address(marketMakingEngine)
);
}
Add above test case to fulFillSwap.t.sol and run with command : forge test --mt test_vault_not_live -vvv.
Impact
the user will lose USDToken to pay the baseFee due to not checking the vault Livens in initialSwap call.
Tools Used
Manual Review
Recommendations
check that the vault is live when user initialSwap.
Updates
Lead Judging Commences
inallhonesty 9 months ago
Submission Judgement Published Assigned finding tags:
`initiateSwap` allows users to initiate swap even when the vault is paused
Prize pool breakdown
Total prize
70,000 USDC
nSLOC
3,25821
USDC / LOC
66,500
3,500
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.