hats-finance / Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4

Other
1 stars 2 forks source link

Wrong "fee" calculation can cause a user to receive zero amount #67

Open hats-bug-reporter[bot] opened 9 months ago

hats-bug-reporter[bot] commented 9 months ago

Github username: -- Twitter username: 97Sabit Submission hash (on-chain): 0x3d7281573f7522cc131a8e44606498a2f7738e86c5a0d81018b020878d40b42e Severity: high

Description: Description\ In the localSwap function, "fee" is calculated as follows:

uint256 fee = FixedPointMathLib.mulWadDown(amount, _vaultFee);

Here's a breakdown of what the above means:

And _vaultFee can only be <=1e18.

In a situation where a user wants to swap 100 tokens and sets minOut to zero. And _vault fee is set to 1e18, such a user will get zero as the output amount without being frontrun.

For example:

Calculating mulWadDown(amount, _vaultFee):

The fee amount will be 100 tokens. And a user gets zero without being frontrun.

  1. Proof of Concept (PoC) File

    https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystVaultVolatile.sol#L572

https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystVaultCommon.sol#L348

reednaa commented 9 months ago

Dublicate of https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/issues/13 (and similar)

Minout can be used to protect against large fee.

ololade97 commented 9 months ago

Dublicate of #13 (and similar)

Minout can be used to protect against large fee.

I checked the the #13 issue. The vulnerability isn't similar to this.

reednaa commented 9 months ago

Please provide arguments.

ololade97 commented 9 months ago

Ok.

So if:

Then:

fee = 100 * 1e18 / 10^18 = 100 tokens

( uint256 fee = FixedPointMathLib.mulWadDown(amount, _vaultFee);)

User gets 0 tokens from the swap

ololade97 commented 9 months ago

I wanted to add. Unlike the frontrunning issue discussed in #13, it's legal for the protocol to set the fee to 1e18 from the start.

So here, I'm not talking about frontrunning. I've pointed out the legal way the protocol operates which could cause users to get 0 swap amount.