Improper Verification of `outgoingTxHash` Leading to Incorrect Refuel Transaction Creation and finalization

[hats-bug-reporter[bot]]

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

Description: Description\ In the startRefuelTxSerializing and finaliseRefuelTxSerializing functions of the VaultBitcoinWallet contract, there is no proper verification of the outgoingTxHash. If the provided outgoingTxHash is not present in the _outboundTxHashToId mapping, it defaults to returning zero. This can lead to the creation of a refuel transaction for outboundTransactions[0], which is incorrect and can cause unintended behavior. The same issue is present in the finaliseRefuelTxSerializing function too.

Attack Scenario\

• Assume there are no outbound transactions with the hash 0xabc123....

• relayer calls the startRefuelTxSerializing function with the hash 0xabc123....

• The _outboundTxHashToId mapping returns 0 because 0xabc123... is not present in the mapping.

• The function proceeds to create a refuel transaction for outboundTransactions[0], which is incorrect and can cause unintended behavior.

note:This issue is also present in the finaliseRefuelTxSerializing

Attachments

1. Proof of Concept (PoC) File

   The vulnerability lies in the startRefuelTxSerializing and finaliseRefuelTxSerializing

function startRefuelTxSerializing(bytes32 outgoingTxHash) public onlyRelayer {
    uint256 _index = _outboundTxHashToId[outgoingTxHash];//@audit-no verification of  outgoingTxHash leads to creation of refuel of outboundTransactions[0]

    OutboundTransaction storage outboundTx = outboundTransactions[_index];
    require(outboundTx.txHash != bytes32(0) && outboundTx.finalisedCandidateHash == bytes32(0), "UOT");

    RefuelTxSerializer _sr = refuelSerializerFactory.createRefuelSerializer(_serializers[_index]);
    _sr.toggleRelayer(msg.sender);

    _refuelSerializers[_index].push(_sr);
    emit RefuelTxStarted(_index, _refuelSerializers[_index].length - 1);
}

function finaliseRefuelTxSerializing(bytes32 outgoingTxHash, uint256 refuelTxId) public onlyRelayer {
    uint256 _index = _outboundTxHashToId[outgoingTxHash];

    OutboundTransaction storage outboundTx = outboundTransactions[_index];
    require(outboundTx.txHash != bytes32(0) && outboundTx.finalisedCandidateHash == bytes32(0), "UOT");

    RefuelTxSerializer _sr = _refuelSerializers[_index][refuelTxId];

    (bytes memory txData, bytes32 txHash) = _sr.getRaw();
    emit RefuelTxBroadcast(txHash, outgoingTxHash, txData);

    require(!outboundTx.refuelCandidatesHashes[txHash], "AA");

    outboundTx.refuelCandidatesHashes[txHash] = true;
    _outboundTxHashToId[txHash] = _index;
}

as we can see in the above functions there is no verification on the outgoingTxHash, leading to potential creation of refuel transactions for incorrect outbound transactions.

2. Revised Code File (Optional)
   • implement proper verification of the outgoingTxHash to ensure it is present in the _outboundTxHashToId mapping before proceeding with the refuel transaction creation. This ensures that the contract remains secure and functions as intended.

[batmanBinary]

@party-for-illuminati ,Could you please provide more details or clarification on why it was deemed invalid?

[party-for-illuminati]

@party-for-illuminati ,Could you please provide more details or clarification on why it was deemed invalid?

https://github.com/hats-finance/illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf/blob/3ad7c2aedf991493aab45d3e0847b7e07f5c0d07/packages/contracts/contracts/illuminex/xengine/chains/btc/wallet/VaultBitcoinWallet.sol#L283

[batmanBinary]

hey @party-for-illuminati ,as i mentioned

The function proceeds to create a refuel transaction for outboundTransactions[0]

here the outboundTransactions[0] will not revert and the check require(outboundTx.txHash != bytes32(0) && outboundTx.finalisedCandidateHash == bytes32(0), "UOT"); passes as the outboundTransactions[0] will have valid outboundTx.txHash and outboundTx.finalisedCandidateHash

can u please verify the same?

[party-for-illuminati]

hey @party-for-illuminati ,as i mentioned

The function proceeds to create a refuel transaction for outboundTransactions[0]

here the outboundTransactions[0] will not revert and the check require(outboundTx.txHash != bytes32(0) && outboundTx.finalisedCandidateHash == bytes32(0), "UOT"); passes as the outboundTransactions[0] will have valid outboundTx.txHash and outboundTx.finalisedCandidateHash

can u please verify the same?

It won't break anything if you create a refuel serializer for an outgoing transaction that is already mined. If you have found that it is possible please provide a PoC

[party-for-illuminati]

Also keep in mind that it checks that finalisedCandidateHash == 0, which is impossible for finished transactions

[batmanBinary]

ok,let me look into it..