hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

Critical Fee Loss in Withdrawal Process #50

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0xa4193cb4bbb0f22a9776d3d01c0187fa13715255bdd87e4eab14659b5681ad70 Severity: high

Description: Description\ Summary The Minter contract's withdrawal mechanism, encompassing the previewWithdrawal and requestWithdrawal functions, suffers from two significant issues: precision loss in fee calculations and inconsistent event emission. These issues can lead to financial discrepancies, potential exploitation, and misleading transaction logs.

Attack Scenario\ Precision Loss in Fee Calculation: The previewWithdrawal function uses integer division to calculate withdrawal fees, resulting in rounding down to zero for small amounts. This can lead to fee-free withdrawals for small amounts, creating an unfair advantage for users who can split large withdrawals into multiple smaller ones.

Impacts

Financial Implications: The protocol may lose significant fee revenue due to users exploiting the precision loss by making multiple small withdrawals. Over time, this could lead to a substantial deficit in expected fees, potentially impacting the protocol's financial stability.

Fairness and User Trust: Users withdrawing larger amounts will effectively pay higher fee rates than those making smaller withdrawals, creating an unfair system. This disparity could erode user trust in the protocol's fee structure and overall fairness.

Illustratiom

wihdrawalFee = 100 FEE_DENOMINATOR = 10000

This means the fee is 1% (100/10000 = 0.01 or 1%).

Let's calculate for different withdrawal amounts:

For a withdrawal of 1000 tokens: feeAmount = 1000 * 100 / 10000 = 10 netAmount = 1000 - 10 = 990

For a withdrawal of 100 tokens: feeAmount = 100 * 100 / 10000 = 1 netAmount = 100 - 1 = 99

For a withdrawal of 10 tokens: feeAmount = 10 * 100 / 10000 = 0.1 which rounds down to 0 netAmount = 10 - 0 = 10

For a withdrawal of 1 token: feeAmount = 1 * 100 / 10000 = 0.01 which rounds down to 0 netAmount = 1 - 0 = 1

As we can see, for small amounts (10 tokens or less in this case), the fee rounds down to 0 due to integer division. This means that small withdrawal effectively pay no fee, which could be exploited by users making multiple small withdrawal to avoid fees.

Attachments

  1. Proof of Concept (PoC) File

    function previewWithdrawal(uint256 amount) public view virtual returns (uint256) {
        uint256 feeAmount = amount*withdrawalFee/FEE_DENOMINATOR;
        uint256 netAmount = amount-feeAmount;
        return netAmount;
    }
  2. Revised Code File (Optional)

    function previewWithdrawal(uint256 amount) public view virtual returns (uint256) {
    uint256 feeAmount = (amount * withdrawalFee + FEE_DENOMINATOR - 1) / FEE_DENOMINATOR;
    uint256 netAmount = amount - feeAmount;
    return netAmount;
    }

    Use ceiling division in previewWithdrawal to ensure accurate fee calculation even for small amounts.

0xRizwan commented 1 month ago

Duplicate of #7 and #48