Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Fees are not sent to their respective recipients when dealing with low decimals tokens

greed

Summary

In order to handle low decimals tokens everywhere in the protocol, Zaros starts by scaling such token amount to 18 decimals so rounding errors are avoided.

When users adjust their positions, a part of their trades are collected as fees and sent to the fee recipients. There are 3 types of fees :

  • settlement fees

  • order fees

  • PnL fees

The fees are collected when an account is being liquidated or an order is fulfilled, in the SettlementBranch::_fillOrder() and LiquidationBranch::liquidateAccounts() functions respectively.

These 2 functions use another interal function called TradingAccount::deductAccountMargin() where all the logic is implemented.

This function will loop through all the allowed collateral and give the due scaled USD value to withdrawMarginUsd()

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/TradingAccount.sol#L528-L566

if (settlementFeeUsdX18.gt(UD60x18_ZERO) && ctx.settlementFeeDeductedUsdX18.lt(settlementFeeUsdX18)) {
// attempt to deduct from this collateral difference between settlement fee
// and amount of settlement fee paid so far
> (ctx.withdrawnMarginUsdX18, ctx.isMissingMargin) = withdrawMarginUsd(
self,
collateralType,
ctx.marginCollateralPriceUsdX18,
settlementFeeUsdX18.sub(ctx.settlementFeeDeductedUsdX18),
feeRecipients.settlementFeeRecipient
);
​
// update amount of settlement fee paid so far by the amount
// that was actually withdraw from this collateral
ctx.settlementFeeDeductedUsdX18 = ctx.settlementFeeDeductedUsdX18.add(ctx.withdrawnMarginUsdX18);
}

In withdrawMarginUsd(), before transferring the tokens to the recipients, the scaled token amount will be down scaled using the MarginCollateralConfiguration::convertUd60x18ToTokenAmount() function.

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/TradingAccount.sol#L434-L469

withdraw(self, collateralType, requiredMarginInCollateralX18);
amountToTransfer =
marginCollateralConfiguration.convertUd60x18ToTokenAmount(requiredMarginInCollateralX18);
​
IERC20(collateralType).safeTransfer(recipient, amountToTransfer);
​
withdrawnMarginUsdX18 = amountUsdX18;
isMissingMargin = false;

Vulnerability Details

The issue is that the given requiredMarginInCollateralX18 is so small, when the downscaling occurs, the returned token amount will equal 0.

When dealing with WBTC for example, the requiredMarginInCollateralX18 will be divided by 1e10 which will round to 0.

So the contract will transfer 0 WBTC to the fee recipients, leaving this fee in the contract but still reducing the trader's account margins as if the transfer actually happened.

Currently, the supported tokens allow 0 amount transfers but the protocol should keep in mind that some tokens revert when attempting to transfer zero tokens.

This coded PoC can be pasted in test\integration\perpetuals\perp-market-branch\getFundingRate\getFundingRate.t.sol and demonstrates a user performing various operations that should be collecting fees. However, the recipients never receive them.

function testBypassFeesAndDustStuck() external {
uint256 btcBalance = 10e8;
​
deal({ token: address(wBtc), to: users.naruto.account, give: btcBalance });
​
vm.startPrank(users.naruto.account);
uint128 tradingAccountId = createAccountAndDeposit(btcBalance, address(wBtc));
int128 narutoPosSizeDelta = int128(uint128(btcBalance));
​
// recipients start with no WBTC
assertEq(wBtc.balanceOf(users.orderFeeRecipient.account), 0);
assertEq(wBtc.balanceOf(users.settlementFeeRecipient.account), 0);
assertEq(wBtc.balanceOf(users.liquidationFeeRecipient.account), 0);
​
// various operations are performed over time which should generate fees
openManualPosition(ETH_USD_MARKET_ID, ETH_USD_STREAM_ID, MOCK_ETH_USD_PRICE, tradingAccountId, int128(ETH_USD_MIN_TRADE_SIZE) * 1_000);
​
skip(5 days);
openManualPosition(ETH_USD_MARKET_ID, ETH_USD_STREAM_ID, MOCK_ETH_USD_PRICE, tradingAccountId, int128(ETH_USD_MIN_TRADE_SIZE) * 1_000);
updateMockPriceFeed(ETH_USD_MARKET_ID, MOCK_ETH_USD_PRICE - (MOCK_ETH_USD_PRICE / 10)); // price drops -10%
skip(5 days);
// user closes his position
openManualPosition(ETH_USD_MARKET_ID, ETH_USD_STREAM_ID, MOCK_ETH_USD_PRICE, tradingAccountId, -int128(ETH_USD_MIN_TRADE_SIZE * 2_000));
​
assertEq(wBtc.balanceOf(address(perpsEngine)), btcBalance);
​
// user withdraws the maximum collateral he can
perpsEngine.withdrawMargin(tradingAccountId, address(wBtc), 999999999);
​
// user retrieves almost his whole collateral, leaving 1 wei of dust in the contract
assertEq(wBtc.balanceOf(address(perpsEngine)), 1);
assertEq(wBtc.balanceOf(users.naruto.account), 999999999);
​
// recipients earned no fees
assertEq(wBtc.balanceOf(users.orderFeeRecipient.account), 0);
assertEq(wBtc.balanceOf(users.settlementFeeRecipient.account), 0);
assertEq(wBtc.balanceOf(users.liquidationFeeRecipient.account), 0);
}

Impact

Fee recipients do not receive their due and these fees end up stuck in the contract.

Tools Used

Manual review

Recommendations

The issue can hardly be patched because the fee percentages are so small, the downscaling will most likely round to 0.

An idea when this happens would be to collect a flat amount of tokens.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

convertUd60x18ToTokenAmount heavily truncates

Appeal created

blckhv Auditor
over 1 year ago
cryptomoon Judge
over 1 year ago
draiakoo Auditor
over 1 year ago
infect3d Auditor
over 1 year ago
blckhv Auditor
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
slavcheww Auditor
over 1 year ago
cryptomoon Judge
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

convertUd60x18ToTokenAmount heavily truncates

Support

FAQs

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