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

[M-05] Excessive Fees and DoS in getFeesForAssetsAmountOut

jakefromstatefarm

Summary

The getFeesForAssetsAmountOut function in the StabilityBranch contract is vulnerable due to its reliance on the external priceX18 parameter without sufficient validation which can lead to excessive fees or denial of service.

Vulnerability Details

The function calculates fees based on the priceX18 parameter, which is externally provided. If priceX18 is manipulated to be extremely low, the calculation baseFeeX18 = ud60x18(tokenSwapData.baseFeeUsd).div(priceX18) can result in an excessively high baseFeeX18, disproportionately increasing the fees charged for swaps. This could make swaps prohibitively expensive or cause the contract to revert transactions due to arithmetic overflow; if priceX18 is set to zero, it would cause a division by zero error, leading to a denial of service by halting swap operations.

function getFeesForAssetsAmountOut( UD60x18 assetsAmountOutX18, UD60x18 priceX18 ) public view returns (UD60x18 baseFeeX18, UD60x18 swapFeeX18) { // load swap data UsdTokenSwapConfig.Data storage tokenSwapData = UsdTokenSwapConfig.load(); // convert the base fee in usd to the asset amount to be charged baseFeeX18 = ud60x18(tokenSwapData.baseFeeUsd).div(priceX18); // calculates the swap fee portion rounding up swapFeeX18 = Math.divUp( assetsAmountOutX18.mul(ud60x18(tokenSwapData.swapSettlementFeeBps)), ud60x18Convert(Constants.BPS_DENOMINATOR) ); }

POC

The priceX18 is sourced from an untrusted input and manipulating priceX18 to be extremely low or zero.

function testExcessiveFeesDueToLowPrice() public {
UD60x18 lowPrice = ud60x18(1); // low price
UD60x18 assetsAmountOutX18 = ud60x18(1000);
​
// Expect the function to revert due to excessive fees
vm.expectRevert();
getFeesForAssetsAmountOut(assetsAmountOutX18, lowPrice);
}

Recommendations

Implement validation checks for the priceX18 parameter to ensure it's within a reasonable range and not zero before performing calculations.

function getFeesForAssetsAmountOut(
UD60x18 assetsAmountOutX18,
UD60x18 priceX18
)
public
view
returns (UD60x18 baseFeeX18, UD60x18 swapFeeX18)
{
// Validate priceX18
require(priceX18 > 0, "Price cannot be zero");
require(priceX18 >= MIN_PRICE && priceX18 <= MAX_PRICE, "Price out of range");
​
// Current logic...
UsdTokenSwapConfig.Data storage tokenSwapData = UsdTokenSwapConfig.load();
baseFeeX18 = ud60x18(tokenSwapData.baseFeeUsd).div(priceX18);
swapFeeX18 = Math.divUp(
assetsAmountOutX18.mul(ud60x18(tokenSwapData.swapSettlementFeeBps)),
ud60x18Convert(Constants.BPS_DENOMINATOR)
);
}

Define MIN_PRICE and MAX_PRICE constants to set acceptable bounds for priceX18 and ensure priceX18 is greater than zero to prevent division by zero errors.

Updates

Lead Judging Commences

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

Support

FAQs

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