Potential Hash Collision in _updateKey Function in VaultBitcoinWallet.sol

[hats-bug-reporter[bot]]

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

Description: Description\ In the VaultBitcoinWallet.sol contract, the _updateKey function is called with a hash generated using keccak256 and abi.encodePacked. The variables being encoded include dynamic types, which can lead to potential hash collisions.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

   _updateKey(keccak256(abi.encodePacked(
   value,
   _vaultScriptHash,
   _recoveryData,
   block.number
   )));

   Using abi.encodePacked for dynamic variables can cause hash collisions because it concatenates the encoded data without any delimiters. This can lead to ambiguous results when decoding.

Consider the following two sets of variables:

1. value = 1, _vaultScriptHash = "abc", _recoveryData = "def"

2. value = 12, _vaultScriptHash = "a", _recoveryData = "bcdef" Both could produce the same concatenated result when using abi.encodePacked, leading to the same hash: abi.encodePacked(1, "abc", "def") -> 0x01abc...def abi.encodePacked(12, "a", "bcdef") -> 0x0ca...bcdef

3. Revised Code File (Optional)

   _updateKey(keccak256(abi.encode(
   value,
   _vaultScriptHash,
   _recoveryData,
   block.number
   )));

[batmanBinary]

hey @rotcivegaf @party-for-illuminati ,please verify the below POC attached

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";

contract HashCollisionTest is Test {
    function testHashCollision() public view {
        // These values are crafted to cause a hash collision
        uint64 value1 = 1;
        uint64 value2 = 1;
        bytes memory vaultScriptHash1 = abi.encodePacked("f0602864f83099a47977494683128fa482dfa124");
        bytes memory vaultScriptHash2 = abi.encodePacked("f06028");
        bytes memory recoveryData1 = abi.encodePacked("def");
        bytes memory recoveryData2 = abi.encodePacked("64f83099a47977494683128fa482dfa124def");

        // Hashing the first set of variables
        bytes32 hash1 = keccak256(abi.encodePacked(value1, vaultScriptHash1, recoveryData1, block.number));
        // Hashing the second set of variables
        bytes32 hash2 = keccak256(abi.encodePacked(value2, vaultScriptHash2, recoveryData2, block.number));

        // Assert that the hashes are equal, indicating a collision
        assertEq(hash1, hash2, "Hash collision did not occur as expected");
    }
}

forge test --match-test testHashCollision -vvvvv

[rotcivegaf]

This PoC has no relation to the protocol contracts