search

hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

Potential locking of funds when the sent BTC does not cover the fee. #83

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

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

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

Description: Description\

This issue comes due to how the current deposit process is done and no minimum deposit requirement.

The current deosit process is,

  1. user create deposit request - deposit address is generated by the relayer.
  2. user send the XBTC (as mentioned in the diagram, fee is not considered) to that address.
  3. User sent the Tx hash to the relayer.
  4. Prover proves the deposit Tx to the Vault.
  5. Inside the VaultBitcoinWallet, the btc_token is minted.

If we see the btc token minting process, fee charging is done like protocol fee, importFees. The importFees is dynamic which acts based on the satoshiPerByte which is set by the owner.

When user deosit any BTC that does not cover these fee value, then BTC will be stuck and the VaultBitcoinWallet can not mint the btc token.

Following two aspects is root cause of this issue.

Impact\

As there are no min deposit limit, any BTC can be sent to mint the btc token. If the sent BTC does not cover the fees, the deposit can not be completed and user's BTC could stuck.

Attachments

  1. Revised Code File (Optional)

In order to ensure smooth minting of btc tokne we would suggest the following.

When depositing the XBTC consider the fee (XBTC + Fee) and include this fee in the Tx hash.

When processing the deposit, deduct this fee from the total BTC and then proceed the minting of btc token.

party-for-illuminati commented 4 months ago

The minimum deposit amount is regulated on the frontend side

aktech297 commented 4 months ago

The minimum deposit amount is regulated on the frontend side

We don't know this details. Readme also didn't mention anything about this.

We saw that the withdraw is restricted with min value in the contract level. We were thinking that this is missed during deposit. It would be helpful if you could add such details in readme. So that we can focus on other potential vulnerabilities.

aktech297 commented 4 months ago

@rotcivegaf

rotcivegaf commented 4 months ago

@aktech297 you report don't provide the lines of the bug, and don't have any PoC

aktech297 commented 4 months ago

@aktech297 you report don't provide the lines of the bug, and don't have any PoC

Hi @rotcivegaf , Sponsor understood the submission and says the min deposit limit is enforced on the front end.

If we see the withdraw function , there is cap for minWithdrawalLimit. but if we see the deposit function, there is no such cap.

as we reported, it is not recommended to enforce it in the deposit function since the BTC is already deposited by the user.

By considering the above point, we suggested to include this with user deposit transaction.

sponsor says, it is enforced in the front end.

But, it will not save the user who directly interact with smart contract.