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

minAmountOut in initiateSwap does not take into account the protocol fee, which may result in canceled swap

elolpuer

Summary

In StabilityBranch.sol:initiateSwap(), the calculation of expectedAmountOut considers only the current vault assets, debt, and amountIn, but ignores protocol fees. Though in StabilityBranch.sol:fulfillSwap(), contract subtracts fees from amountOut and verifies that amountOut - fees does not exceed the initially expected amountOut.

Vulnerability Details

In initiateSwap() contract does not take into account protocol fees for amountOut check:

function initiateSwap(
uint128[] calldata vaultIds,
uint128[] calldata amountsIn,
uint128[] calldata minAmountsOut
)
external
{
...
for (uint256 i; i < amountsIn.length; i++) {
...
ctx.expectedAssetOut =
getAmountOfAssetOut(vaultIds[i], ud60x18(amountsIn[i]), ctx.collateralPriceX18).intoUint256();
​
// revert if the slippage wouldn't pass or the expected output was 0
if (ctx.expectedAssetOut == 0) revert Errors.ZeroOutputTokens();
//@audit check without fees
if (ctx.expectedAssetOut < minAmountsOut[i]) {
revert Errors.SlippageCheckFailed(minAmountsOut[i], ctx.expectedAssetOut);
}
...
}
}

But in fulfillSwap() contract subtracts fees from amountOut for the check:

function fulfillSwap(
address user,
uint128 requestId,
bytes calldata priceData,
address engine
)
external
onlyRegisteredSystemKeepers
{
...
ctx.amountOutBeforeFeesX18 = getAmountOfAssetOut(ctx.vaultId, ud60x18(ctx.amountIn), ctx.priceX18);
​
// gets the base fee and swap fee for the given amount out before fees
(ctx.baseFeeX18, ctx.swapFeeX18) = getFeesForAssetsAmountOut(ctx.amountOutBeforeFeesX18, ctx.priceX18);
...
// subtract the fees and convert the UD60x18 value to the collateral's decimals value
//@audit but in fulfill swap contract now checks also fees
ctx.amountOut =
collateral.convertUd60x18ToTokenAmount(ctx.amountOutBeforeFeesX18.sub(ctx.baseFeeX18.add(ctx.swapFeeX18)));
​
// slippage check
ctx.minAmountOut = request.minAmountOut;
if (ctx.amountOut < ctx.minAmountOut) {
revert Errors.SlippageCheckFailed(ctx.minAmountOut, ctx.amountOut);
}
}

Impact

The user may omit fees when specifying amountOut, but the contract will still allow them to initiate swap. In result will be cancelled swap and user will have to pay baseFee to refund.

Tools Used

Manual Review

Recommendations

Take in to account protocol fees for amountOut check in initiateSwap().

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Missing basefee and settlement fee deduction before slippage check in the Stablitybranch contract.

Support

FAQs

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

Give us feedback!