Potential Hash Collision of `keccak256` with `abi.encodePacked` in `packages/contracts/contracts/illuminex/xengine/chains/btc/wallet/VaultBitcoinWallet.sol::_onActionDeposit` function.

[hats-bug-reporter[bot]]

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0xfee3fb89e9b9d31f3078141aba9aba0c17c49198840f5d393a96639d07b77da2 Severity: low

Description: Description

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead

Attack Scenario

Hash collision for different inputs which might put protocol in an unexpected state.

Attachments

NA

1. Proof of Concept (PoC) File

In packages/contracts/contracts/illuminex/xengine/chains/btc/wallet/VaultBitcoinWallet.sol::_onActionDeposit:

    function _onActionDeposit(
        uint64 value,
        bytes memory _vaultScriptHash,
        bytes memory _recoveryData
    ) internal returns (bytes32) {
        //REDACTED

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

        //REDACTED

_vaultScriptHash & _recoveryData are dynamic bytes memory type which can create hash collision due to abi.encodePacked.

2. Revised Code File (Optional)

Making the following changes in packages/contracts/contracts/illuminex/xengine/chains/btc/wallet/VaultBitcoinWallet.sol::_onActionDeposit:

    function _onActionDeposit(
        uint64 value,
        bytes memory _vaultScriptHash,
        bytes memory _recoveryData
    ) internal returns (bytes32) {
        //REDACTED

-- _updateKey(keccak256(abi.encodePacked(
++ _updateKey(keccak256(abi.encode(

            value,
            _vaultScriptHash,
            _recoveryData,
            block.number
        )));

        //REDACTED

[rotcivegaf]

Non issue