Protocol Fees Calculation Precision Issue

[hats-bug-reporter[bot]]

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

Description: Description\ The withdraw function in the VaultBitcoinWallet contract has a precision issue when calculating protocol fees. Due to Solidity's integer division, amounts can result in the protocolFees being rounded down to zero, which is not the intended behavior.

Attack Scenario\ let's examine this with example

amount=1000

minReceiveAmount=750

BYTES_PER_OUTGOING_TRANSFER=30

satoshiPerByte=1

now by calling withdraw() function with the above paramters users can bypass protocol fees

amountAfterNetworkFee:

amountAfterNetworkFee = amount - (BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte);

amountAfterNetworkFee = 1000-30 = 970

protocolFees:

uint64 protocolFees = amountAfterNetworkFee * withdrawalFee / 1000;

 protocolFees = 970*1/1000 = 0

as we can see here by using this anyuser can bypass protocol fees

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
           )
       );
   }

2. Revised Code File (Optional)

   uint64 protocolFees = amountAfterNetworkFee * withdrawalFee / 1000;
       if (isExcludedFromFees[msg.sender]) {
           protocolFees = 0;
       } else if ( protocolFees <= 1) {
           protocolFees = 1; // Ensure protocol fees are at least 1 satoshi if non-zero
       }

[rotcivegaf]

Dup of: https://github.com/hats-finance/illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf/issues/23

[AresAudits]

hey @rotcivegaf , #23 is invalid as the scenario mentioned always reverts

[AresAudits]

@party-for-illuminati ?

[rotcivegaf]

1 sathoshi...

[party-for-illuminati]

@party-for-illuminati ?

This report is correctly labelled as invalid

[AresAudits]

hey @rotcivegaf @party-for-illuminati , i agree with your decision.just curious why the issue is invalid(for feedback)

[rotcivegaf]

In this case the protocol at most can lose 1 wei(satoshi) per transaction, because there is a minWithdrawalLimit so the amount should be greather than amount + (BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte)

Finally the amount can't be 1000 and at most the protocol lose 1 wei at max, also if the amount can be 1000

PD.: Read this report: the-graph-rounding-error

[IlIlHunterlIlI]

i just want to elaborate that 1 satoshi is lost from a (1e8) bitcoin (which is not a dust value) and will cause leak of value on the long run

[AresAudits]

i agree with @IlIlHunterlIlI ,

i just want to elaborate that 1 satoshi is lost from a (1e8) bitcoin (which is not a dust value) and will cause leak of value on the long run

If we calculate the loss practically (this can be exploited by anyone as the difficulty for exploitation is very low):

• Batching interval: 15 minutes
• Total minutes in a day: 24×60 = 1440 minutes

Now,

• Total number of transactions/transfers per day: (1440/15)*5 = 480

Max Loss of protocol fees:

• Per day : 480 * 1 Satoshi = 480 Satoshi = $261.35 USD
• Per month : 480*30 = 14400 Satoshi = $7840 USD
• Per Year : 14400*12 = 172,800 Satoshi = $94086.50 USD

As we can see, the protocol will incur significant losses due to the rounding issue, and these losses will compound over time.

@party-for-illuminati waiting for your response on this..

[batmanBinary]

@party-for-illuminati ?