Submission Details
Severity:
Missing basefee and settlement fee deduction before slippage check in the Stablitybranch contract.
Summary
Each swap initiated in the stability contract has a slippage check to prevent users from losing funds and ensuring they get the optimal value but this check during initiation does not ensure that the removal of fees will not revert the transaction.
Also if these fees were removed and the swap is good it should proceed but because the amount out should to checked if this is possible is bigger than the actual amount out the call will revert a good swap.
Vulnerability Details
Ideally, the check should subtract the base fee and settlement fee but this is not done during the swap simulation.
/// @notice Initiates multiple (or one) USD token swap requests for the specified vaults and amounts.
/// @param vaultIds An array of vault IDs from which to take assets.
/// @param amountsIn An array of USD token amounts to be swapped from the user in zaros internal precision
/// @param minAmountsOut An array of minimum acceptable amounts of collateral the user expects to receive for each
/// swap.
/// @dev Swap is fulfilled by a registered keeper.
/// @dev Invariants involved in the call:
/// The arrays lengths MUST match
/// The vaults MUST be of the same asset
function initiateSwap( //@AUDIT batching isssue NOTE
uint128[] calldata vaultIds,
uint128[] calldata amountsIn,
@audit>> uint128[] calldata minAmountsOut
)
external
{
// Perform length checks
if (vaultIds.length != amountsIn.length) {
revert Errors.ArrayLengthMismatch(vaultIds.length, amountsIn.length);
}
if (amountsIn.length != minAmountsOut.length) {
revert Errors.ArrayLengthMismatch(amountsIn.length, minAmountsOut.length);
}
// working data
InitiateSwapContext memory ctx;
// cache the vault's index token and asset addresses
Vault.Data storage currentVault = Vault.load(vaultIds[0]);
ctx.initialVaultIndexToken = currentVault.indexToken;
ctx.initialVaultCollateralAsset = currentVault.collateral.asset;
// load collateral data; must be enabled
Collateral.Data storage collateral = Collateral.load(ctx.initialVaultCollateralAsset);
collateral.verifyIsEnabled();
// load market making engine config
MarketMakingEngineConfiguration.Data storage configuration = MarketMakingEngineConfiguration.load();
// load usd token swap data
UsdTokenSwapConfig.Data storage tokenSwapData = UsdTokenSwapConfig.load();
// cache additional common fields
// ctx.collateralPriceX18 in zaros internal precision
ctx.collateralPriceX18 = currentVault.collateral.getPrice();
ctx.maxExecTime = uint120(tokenSwapData.maxExecutionTime);
// ctx.vaultAssetBalance in native precision of ctx.initialVaultCollateralAsset
ctx.vaultAssetBalance = IERC20(ctx.initialVaultCollateralAsset).balanceOf(ctx.initialVaultIndexToken);
for (uint256 i; i < amountsIn.length; i++) {
// for all but first iteration, refresh the vault and enforce same collateral asset
if (i != 0) {
currentVault = Vault.load(vaultIds[i]);
// revert for swaps using vaults with different collateral assets
if (currentVault.collateral.asset != ctx.initialVaultCollateralAsset) {
revert Errors.VaultsCollateralAssetsMismatch();
}
// refresh current vault balance in native precision of ctx.initialVaultCollateralAsset
ctx.vaultAssetBalance = IERC20(ctx.initialVaultCollateralAsset).balanceOf(currentVault.indexToken); // NOTE @audit user can initiate a swap that will fail, track balance correctly medium. user loses funds, base fee.
}
// cache the expected amount of assets acquired with the provided parameters
// amountsIn[i] and ctx.collateralPriceX18 using zaros internal precision
@audit>> ctx.expectedAssetOut =
getAmountOfAssetOut(vaultIds[i], ud60x18(amountsIn[i]), ctx.collateralPriceX18).intoUint256(); //@AUDIT NOTE
// revert if the slippage wouldn't pass or the expected output was 0
if (ctx.expectedAssetOut == 0) revert Errors.ZeroOutputTokens();
@audit>> if (ctx.expectedAssetOut < minAmountsOut[i]) {
revert Errors.SlippageCheckFailed(minAmountsOut[i], ctx.expectedAssetOut);
}
// if there aren't enough assets in the vault to fulfill the swap request, we must revert
@audit>> if (ctx.vaultAssetBalance < ctx.expectedAssetOut) { // WRONG CHECK note
revert Errors.InsufficientVaultBalance(vaultIds[i], ctx.vaultAssetBalance, ctx.expectedAssetOut);
}
Issue 1. slippage will not handle the swap correctly
// revert if the slippage wouldn't pass or the expected output was 0
if (ctx.expectedAssetOut == 0) revert Errors.ZeroOutputTokens();
@audit>> check can pass here and fail in swap >> if (ctx.expectedAssetOut < minAmountsOut[i]) {
revert Errors.SlippageCheckFailed(minAmountsOut[i], ctx.expectedAssetOut);
}
In swap
// subtract the fees and convert the UD60x18 value to the collateral's decimals value
@audit>> ctx.amountOut =
collateral.convertUd60x18ToTokenAmount(ctx.amountOutBeforeFeesX18.sub(ctx.baseFeeX18.add(ctx.swapFeeX18))); //test this against convert individually to colateral and substracting
// slippage check
ctx.minAmountOut = request.minAmountOut;
@audit>> if (ctx.amountOut < ctx.minAmountOut) { //can revert because of 1 wei that is not accounted for ..... NOTE. 1000<1000.000000000001 see line 387 for test
revert Errors.SlippageCheckFailed(ctx.minAmountOut, ctx.amountOut);
}
Issue 2. a good swap will fail
Only about 90% of the settlement fee will be transferred after the swap.
// calculates the protocol's share of the swap fee by multiplying the total swap fee by the protocol's fee
// recipients' share.
@audit>> max fee is 0.9e18>> ctx.protocolSwapFeeX18 = ctx.swapFeeX18.mul(ud60x18(marketMakingEngineConfiguration.totalFeeRecipientsShares));
// the protocol reward amount is the sum of the base fee and the protocol's share of the swap fee
ctx.protocolReward = collateral.convertUd60x18ToTokenAmount(ctx.baseFeeX18.add(ctx.protocolSwapFeeX18));
// update vault debt
vault.marketsRealizedDebtUsd -= int128(ctx.amountIn);
// burn usd amount from address(this)
ctx.usdToken.burn(ctx.amountIn);
// transfer the required assets from the vault to the mm engine contract before distributions
// note: as the swap fee stays in the ZLP Vault, it is technically a net gain to share holders, i.e it is auto
// accumulated to the contract
@audit>> IERC20(ctx.asset).safeTransferFrom(vault.indexToken, address(this), ctx.amountOut + ctx.protocolReward); // @audit we are sending less actually 1 wei less // AUDIT WE CAN GO BELOW THE LOCKED ASSET CHECK IN WITHDRAW.
// distribute protocol reward value
@audit>> marketMakingEngineConfiguration.distributeProtocolAssetReward(ctx.asset, ctx.protocolReward);
Hence the check-in initiate will revert to a good swap call, even though we have enough.
// if there aren't enough assets in the vault to fulfill the swap request, we must revert
if (ctx.vaultAssetBalance < ctx.expectedAssetOut) { // WRONG CHECK note
revert Errors.InsufficientVaultBalance(vaultIds[i], ctx.vaultAssetBalance, ctx.expectedAssetOut);
}
Impact
An incorrect slippage check will cause a user to initiate a swap that will reverse during execution and note the user will still pay the basefee regardless. Two, a good swap will be reversed because of the strict amount to balance check without the properly accessing Reward amount and user amount-out that we will be sending out.
Tools Used
manual review
Recommendations
include the fee in the slippage check during initiation and also ensure we correctly track the amount needed against the contract balance.
Updates
Lead Judging Commences
inallhonesty 3 months ago
Submission Judgement Published Assigned finding tags:
Missing basefee and settlement fee deduction before slippage check in the Stablitybranch contract.
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.