Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing zero amount check in requestWithdrawal #72

Open
hats-bug-reporter bot opened this issue Sep 16, 2024 · 2 comments
Open

Missing zero amount check in requestWithdrawal #72

hats-bug-reporter bot opened this issue Sep 16, 2024 · 2 comments
Labels
Invalid - lead auditor invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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...
}
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 16, 2024
@0xRizwan
Copy link

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
Copy link

ilzheev commented Sep 16, 2024

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

@ilzheev ilzheev added invalid This doesn't seem right and removed bug Something isn't working labels Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid - lead auditor invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants