hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

Incorrect Handling of Insufficient Amounts in withdraw Function #56

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x6325c186e930458035fb83e62bd79d10e816a37ae8ba36a14d94130eb2de4384 Severity: low

Description: Description\ The withdraw function in the VaultBitcoinWallet contract does not correctly handle cases where the amount is insufficient to cover both the network fee and the protocol fees. This can potentially lead to an underflow Reverts or Dos. and not to forget that satoshiPerByte and minWithdrawalLimit has no upper limit.


    function setFee(uint64 _satoshiPerByte) public {
        require(msg.sender == feeSetter);
        emit FeeSet(_satoshiPerByte);

        satoshiPerByte = _satoshiPerByte;
    }

therefore in real world scenario amount can be equal to BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File

    function withdraw(bytes memory to, uint64 amount, uint64 minReceiveAmount, bytes32 idSeed) public {
    uint64 amountAfterNetworkFee = amount - (BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte);
    require(amountAfterNetworkFee >= minWithdrawalLimit, "AFL");
    
    uint64 protocolFees = amountAfterNetworkFee * withdrawalFee / 1000;
    if (isExcludedFromFees[msg.sender]) {
        protocolFees = 0;
    }
    
    uint64 amountAfterFee = amountAfterNetworkFee - protocolFees;
    require(amountAfterFee >= minReceiveAmount, "FTH");
    
    btcToken.burn(msg.sender, amount);
    if (protocolFees > 0) {
        btcToken.mint(owner(), protocolFees);
    }
    
    bytes32 _transferId = keccak256(abi.encodePacked(idSeed, to, amount));
    queue.push(
        OutgoingQueue.OutgoingTransfer(
            BitcoinUtils.resolveLockingScript(to, _isTestnet(), workingScriptSet),
            amountAfterFee,
            _transferId
        )
    );
    }

    Example Scenario Demonstrating the Issue

  2. Network Fee Calculation: networkFee = BYTES_PER_OUTGOING_TRANSFER satoshiPerByte networkFee = 30 2 = 60 satoshis

  3. Amount After Network Fee: amountAfterNetworkFee = amount - networkFee amountAfterNetworkFee = 700 - 60 = 640 satoshis

  4. Check Against minWithdrawalLimit: require(amountAfterNetworkFee >= minWithdrawalLimit, "AFL") 640 < 700 (This check fails)

  5. Revised Code File (Optional)

    
    function withdraw(bytes memory to, uint64 amount, uint64 minReceiveAmount, bytes32 idSeed) public {
        uint64 networkFee = BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte;
        require(amount >= networkFee, "Amount less than network fee");

* Network Fee Check: Ensures amount is sufficient to cover the network fee.
rotcivegaf commented 3 months ago

Is the expect behavior