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

FoT tokens not supported, registered amount will not take fee from token transfer into account

infect3d

Summary

User depositing a fee-on-transfer (FoT) token to its account will see its account credited with the complete amount, while it should only be credited by amount - fee, as the protocol will not receive the fee part.\

This is also the case for stETH because of its 1-2 wei corner case even though here amount are less, this will still break the accounting.

Vulnerability Details

The TradingAccountBranch::depositMargin function allow users to deposit tokens to an account.
The function takes an amount parameter which is then converted to the protocol decimal system L328 as amountX18
The issue happen L346 and L349.
For FoT tokens, The safeTransferFrom will decrease user balance by amount, but the recipient will receive amount - fee
Then, tradingAccount is credited with amountX18 which do not take the fee into account.

File: src/perpetuals/branches/TradingAccountBranch.sol
317: function depositMargin(uint128 tradingAccountId, address collateralType, uint256 amount) public virtual {
...:
...: //* ... some code ... *//
...:
327: // convert uint256 -> UD60x18; scales input amount to 18 decimals
328: UD60x18 amountX18 = marginCollateralConfiguration.convertTokenAmountToUd60x18(amount); //* here probably using that function bc amount is in tokens' decimals
...:
...: //* ... some code ... *//
...:
345: // get the tokens first
346: IERC20(collateralType).safeTransferFrom(msg.sender, address(this), amount);
347:
348: // then perform the actual deposit
349: tradingAccount.deposit(collateralType, amountX18);
350:
351: emit LogDepositMargin(msg.sender, tradingAccountId, collateralType, amount);
352: }

Impact

Wrong accounting of protocol owned assets compared to users balances.

Tools Used

Manual review

Recommendations

Credit the real received amount to the user rather than the raw amount.
This can be done by measuring contract balance before and after the transfer, and credit the difference.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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