search

code-423n4 / 2024-06-krystal-defi-findings

0 stars 0 forks source link

User sets the fees himself #19

Closed c4-bot-2 closed 2 months ago

c4-bot-2 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/V3Utils.sol#L105-L106 https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/V3Utils.sol#L209-L214 https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/V3Utils.sol#L254-L255

Vulnerability details

Impact

User can avoid paying protocol fees in V3Utils.

Proof of Concept

In V3Utils the protocol fees are deducted by Common._deductFees() according to a user supplied parameter protocolFeeX64, in onERC721Received(), ´in swapAndMint()´ and ´in swapAndIncreaseLiquidity()´. The only restriction placed on this is that it not surpass the _maxFeeX64. The user is thus free to set it to 0, avoiding all fees.

Recommended Mitigation Steps

Have a fixed fee in Common._deductFees() set by the admin.

Assessed type

Other

c4-judge commented 2 months ago

3docSec marked the issue as satisfactory

c4-judge commented 2 months ago

3docSec marked the issue as selected for report

c4-judge commented 2 months ago

3docSec marked the issue as primary issue

Haupc commented 2 months ago

We only charge fee with users who use our UI to execute txn

Haupc commented 2 months ago

same as #12

3docSec commented 2 months ago

Invalid: intended behavior

c4-judge commented 2 months ago

3docSec marked the issue as unsatisfactory: Invalid