hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

Missing zero amount check in `requestWithdrawal` #72

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): 0x58cab97ada4a4ceecc7889f9ccc4d397548f22649a5ae65b68b936f0c5170a4a Severity: low

Description: Description\ In the requestWithdrawal function of the contract, there is a missing check for a zero net amount after fee calculation. While the function checks if the input amount is greater than or equal to the minimum withdrawal amount, it does not verify if the net amount (after calculating fees) is greater than zero. This is inconsistent with the approach taken in the redeem function, which includes such a check. This oversight could potentially lead to the creation of withdrawal requests with no effective value.

Impact\

While not a high-risk vulnerability, this oversight could lead to the following issues:

  1. Creation of withdrawal requests that represent zero value after fees, leading to unnecessary storage and gas costs.
  2. Inconsistency in contract behavior between withdrawal requests and redemptions.
  3. Potential confusion for users who might create withdrawal requests that effectively represent no value.
  4. In extreme cases with high fees, it could lead to the minting of NFTs representing zero-value withdrawal requests.

Recomenadtion\

To address this issue and maintain consistency with the redeem function, add a check for the net amount after the previewWithdrawal call:

function requestWithdrawal(uint256 amount, address receiver) public nonReentrant {
    require(amount >= minWithdrawal, "LessThanMin");
    uint256 netAmount = previewWithdrawal(amount);
    require(netAmount > 0, "ZeroNetAmount");

    // Rest of the function...
}
0xRizwan commented 2 months ago

minWithdrawal is being checked so netAmount won't be zero. For example, minWithdrawal amount would be 1 token so if we consider max withdrawal fee which is 5% then even such case, netAmount won't be zero. Additionally, netAmount is being added here. on other hand, redeem does not have minimum redeem amount check so redeemAmount is being checked to be greater than 0. This is required to transfer the redeemAmount to receiver address only if its greater than 0 and otherwise revert the redeem.

ilzheev commented 2 months ago

Non-possible in real deployment usage as minWithdrawal is set to some arbitrary amount (not 1 gwei) to avoid spam.