hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

DoS risk and fee manipulation inside `BaseMinterRedeem` through missing safeguards #71

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

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

Github username: @0xMilenov Twitter username: 0xMilenov Submission hash (on-chain): 0xa0ebac217ce3cd765d94c49306af995dab22572b71548aa9541c3686db068669 Severity: medium

Description: Description

The BaseMinterRedeem contract, an extension of BaseMinter, is designed to manage redeem fees for a liquid staking system.

However, it contains several omissions that, when combined, create a significant vulnerability. The contract lacks a minimum redeem amount, a total redeem fees tracking mechanism, and proper reentrancy protection on the previewRedeem function. These oversights can lead to potential DoS attacks, inaccurate fee calculations, and manipulation of the redeem process.

Additionally, the contract is missing a crucial function to collect the accumulated redeem fees, rendering the fee system incomplete.

Attack Scenario

An attacker could exploit this vulnerability through the following steps:

  1. The attacker initiates numerous small redeem requests, each just above zero but below any reasonable minimum amount.
  2. Due to the missing minimum redeem amount check, these transactions are processed.
  3. The previewRedeem function, lacking reentrancy protection, could be called multiple times in a single transaction, potentially leading to race conditions and incorrect fee calculations.
  4. Without a total redeem fees variable, the contract fails to accurately track the accumulated fees, making it difficult for the owner to collect them properly.
  5. The combination of these issues allows the attacker to:
    • a) Congest the network with many small transactions, potentially causing a DoS condition.
    • b) Manipulate the redeem process to pay less in fees than they should.
    • c) Obfuscate the true state of redeem fees, making it challenging for the contract owner to manage the system effectively.

PoC

The vulnerability is pretty straightforward and the exploitable scenario proofs it.

Recommendation

To address this vulnerability, implement the following fixes:

  1. Introduce a minRedeemAmount variable, similar to the minWithdrawal in BaseMinterWithdrawal:

    uint256 public minRedeemAmount = ?; // Set an appropriate minimum value
  2. Add a check for the minimum redeem amount in relevant functions:

    require(amount >= minRedeemAmount, "Amount below minimum");
  3. Implement a totalRedeemFees variable to track accumulated fees:

    uint256 public totalRedeemFees = 0;
  4. Update the redeem process to calculate and add fees to totalRedeemFees:

    uint256 feeAmount = (amount * redeemFee) / FEE_DENOMINATOR;
    totalRedeemFees += feeAmount;
  5. Add a collectRedeemFees function for the owner to withdraw accumulated fees:

    function collectRedeemFees(address receiver) public onlyOwner {
    uint256 feesToCollect = totalRedeemFees;
    totalRedeemFees = 0;
    // Transfer fees to receiver
    }
  6. Apply the nonReentrant modifier to the previewRedeem function:

    function previewRedeem(uint256 amount) public view virtual nonReentrant returns (uint256) {
    // Existing code
    }

    Consider implementing a cooldown period or gas limit for redeem operations to prevent rapid, successive small redeems.

0xRizwan commented 2 months ago

Duplicate of #15

ilzheev commented 2 months ago

Redeem fees are not collected.