The current Design in `VaultBitcoinWallet::minWithdrawalLimit` makes it so easy for malicious persons to DOS the withdrawals

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x0d0579aa17d6e4a153dac4353519ce1e93a9d9108fdd585c78bdbf1bf5c84742 Severity: medium

Description: Description\ The withdraw function in the VaultBitcoinWallet contract has a vulnerability related to the minimum withdrawal limit. The current implementation allows for frequent small withdrawals, which could be exploited to cause a denial of service (DoS) condition for legitimate withdrawals.

Key points:

1. The minWithdrawalLimit is set to 700 satoshis (approximately $0.40 at current BTC prices).
2. The OutgoingQueue contract has a maxTransfersPerBatch of 5 and a batchingInterval of 15 minutes.
3. These parameters allow for frequent small withdrawals that could potentially clog the withdrawal queue.

Attack Scenario\ A malicious actor could exploit this vulnerability as follows:

1. The attacker initiates multiple minimum amount withdrawals (700 satoshis each).

2. They perform 5 withdrawals every 15 minutes, which is the maximum allowed by the OutgoingQueue contract.

3. This attack pattern can be sustained as follows:

• 5 withdrawals per 15 minutes = 20 withdrawals per hour
• 20 withdrawals per hour * 24 hours = 480 withdrawals per day
• 480 withdrawals per day * 30 days = 14,400 withdrawals per month

4. Cost to the attacker:

   • Each withdrawal costs approximately $0.40
   • Daily cost: 14,400 * $0.40 = $5760

5. Impact

• The withdrawal queue becomes clogged with these small transactions.
• Legitimate users may face significant delays in processing their withdrawals.
• The protocol's normal operation is disrupted at a relatively low cost to the attacker.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[party-for-illuminati]

Invalid because minWithdrawalLimit is adjustable by the owner

[IlIlHunterlIlI]

@party-for-illuminati first of all thank you sir for the fast response second, the argument that minWithdrawalLimit is adjustable by the owner is the same as the argument the the upgradable contracts can be upgraded when problems arise

third: i worked with the numbers that will be available upon deployment,

fourth: the 6K needed to DOS the queue for one month aren't the money spent but indeed the money needed to carry the attack

• the lost money are the fee of the network and the protocol from those 6K

knowing the fact the an attacker can clog your whole queue with such a low cost (upon deployment)

would really love to hear from the lead auditor how this doesn't bring any value for the protocol ? @rotcivegaf

not trying to push any thing but i thing the issue is overlocked

[IlIlHunterlIlI]

to show the problem more clearly

• its the fact that minWithdrawalLimit too low combined with maxTransfersPerBatch of 5 and a batchingInterval of 15 minutes.

• upon deployment minWithdrawalLimit is 0.5 $

1. user deposit large amount of funds (ie 1M$)
2. he instantly withdraws them in separate dust requests (2M requests)
3. this basically just block the withdrawal Queue for 11.5 years

for more clarification (i will always be glad to add more)

[IlIlHunterlIlI]

Invalid because minWithdrawalLimit is adjustable by the owner

@party-for-illuminati if a large requests already in the queue, then how adjusting this solves it?

want to add that your pegged BTC will get destroyed in DEFI protocols integrated with it,why?

cause now user submits a withdrawal and it happens to be the 1000,001 index

now they already burned their btc but nearly won't get their correlated BTC on the other chain

which destroys the idea of the peg (now it becomes a virtual peg) since the protocol can't give the btc back

withdrawals being DOSed = being unavailable take a look on examples that failed to deliver the underlying peg

[IlIlHunterlIlI]

you are totally right, adjusting it preDeployement or develop another design that we can discuss is the solution

but in fact you were deploying your code with 700 value

[party-for-illuminati]

to show the problem more clearly

• its the fact that minWithdrawalLimit too low combined with maxTransfersPerBatch of 5 and a batchingInterval of 15 minutes.
• upon deployment minWithdrawalLimit is 0.5 $

1. user deposit large amount of funds (ie 1M$)
2. he instantly withdraws them in separate dust requests (2M requests)
3. this basically just block the withdrawal Queue for 11.5 years

for more clarification (i will always be glad to add more)

minWithdrawalLimit will be changed, owner can change it anytime and it won't be 700 in production

[IlIlHunterlIlI]

@party-for-illuminati would love to hear more about other comments

[party-for-illuminati]

@party-for-illuminati would love to hear more about other comments

Could you elaborate please what would you like me to clarify?

[IlIlHunterlIlI]

@party-for-illuminati

it won't be 700 in production

i think this should end the discussion, i mean don't want to waste your time about severity and attack paths cause you already disregarding the issue with that statement

• If we can't agree its a mistake to put it to 700 then there is no value to talk about other details

• the fact that you don't want to deploy your code with that value clearly elaborate that its a mistake in the code.

i owe all the respect to you (cause words don't have emotions, so don't mistake my words as attacking)

[party-for-illuminati]

@party-for-illuminati

it won't be 700 in production

i think this should end the discussion, i mean don't want to waste your time about severity and attack paths cause you already disregarding the issue with that statement

• If we can't agree its a mistake to put it to 700 then there is no value to talk about other details
• the fact that you don't want to deploy your code with that value clearly elaborate that its a mistake in the code.

i owe all the respect to you (cause words don't have emotions, so don't mistake my words as attacking)

It isn't a mistake. 700 is set as default for testing purposes, it was never meant to be used in production, that's why the owner (multisig) can update it

[IlIlHunterlIlI]

@party-for-illuminati are you familiar with vault inflation attacks? or first depositer attacks?

since you are not changing that variable in the constructor i think the attack is still doable

cause if you are deploying the vault in one txn and change the minWithdrawalLimit in next txn this still gives me time for doing the attack

The attacker can still make a contract that loops through 100K iterations of withdrawing dust amount (before you can change it with your multisig)

[IlIlHunterlIlI]

@party-for-illuminati

[IlIlHunterlIlI]

@party-for-illuminati can i get any heads up about this or you think its invalid and thats it?

[party-for-illuminati]

@party-for-illuminati are you familiar with vault inflation attacks? or first depositer attacks?

since you are not changing that variable in the constructor i think the attack is still doable

cause if you are deploying the vault in one txn and change the minWithdrawalLimit in next txn this still gives me time for doing the attack

The attacker can still make a contract that loops through 100K iterations of withdrawing dust amount (before you can change it with your multisig)

This attack is impossible here because you need to have some BTC deposited first in order to withdraw it, which will allow us to configure those values prior to making it production ready

[IlIlHunterlIlI]

@party-for-illuminati how much are you willing to set it at?

[IlIlHunterlIlI]

Plus, finalization of deposits are automated by relayers? Or how would i get my deposit delayed till you change it? @party-for-illuminati

[party-for-illuminati]

Plus, finalization of deposits are automated by relayers? Or how would i get my deposit delayed till you change it? @party-for-illuminati

It is delayed by the Bitcoin network confirmations per se, as well as relayers would need to submit your transaction to the contract

[IlIlHunterlIlI]

@party-for-illuminati how much are you willing to set it at?

@party-for-illuminati