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

b0g0_ctf - Unlimited deposits lead to potential loss of funds #25

Open
dimi6oni opened this issue Aug 12, 2024 · 0 comments
Open

b0g0_ctf - Unlimited deposits lead to potential loss of funds #25

dimi6oni opened this issue Aug 12, 2024 · 0 comments
Labels
invalid This doesn't seem right

Comments

@dimi6oni
Copy link

Description

The deposit function allows users to deposit multiple times without minting additional NFTs. This can lead to a situation where a user's total deposits exceed the amount they can withdraw.

Impact

Users may lose funds as they can deposit more ETH than they can withdraw, potentially locking excess ETH in the contract permanently.
For example:

  1. User deposits 1 ETH three times (assuming depositRequired is 1 ETH).
  2. They now have 3 NFTs and their deposits[user] balance is 3 ETH.
  3. However, they can only withdraw 1 ETH per NFT, so they can only get back 3 ETH by burning all 3 NFTs.
  4. If they make more deposits than the number of NFTs they receive, they will have locked funds that they cannot withdraw.

Remediation

Implement a check to ensure that a user can only deposit if they don't already have an active deposit, or mint an NFT for each deposit made.

@dimi6oni dimi6oni changed the title Unlimited deposits lead to potential loss of funds b0g0_ctf - Unlimited deposits lead to potential loss of funds Aug 12, 2024
@BogoCvetkov BogoCvetkov added the invalid This doesn't seem right label Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants