Hash collision due to abi.encodePacked

[hats-bug-reporter[bot]]

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

Description: Description\

Solidity docs clearly states abi.encodePacked can result in hash collisions when used with two dynamic arguments.

Attack Scenario\

If keccak256(abi.encodePacked(a, b)) is use and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa.

The result can be abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c").

The issue exists in the Channels, NodeSafeRegistry and Ledger contracts that can results in hash collisions.

Attachments https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L126 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L129 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L427 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L435 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L448 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L496 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L779 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L783 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L839 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L843 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Channels.sol#L871 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/Ledger.sol#L98 https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/node-stake/NodeSafeRegistry.sol#L160

1. Proof of Concept (PoC) File

Out of many function using abi.encodePacked the _isWinningTicket function in Channels.sol can result in signatures to be reused due to hash collisions.

                        abi.encodePacked(
                            // unique due to ticketIndex + ticketEpoch
                            ticketHash,
                            // use deterministic pseudo-random VRF output generated by redeemer
                            params.vx,
                            params.vy,
                            // challenge-response packet sender + next downstream node
                            redeemable.porSecret,
                            // entropy by ticket issuer, only ticket issuer can generate a valid signature
                            redeemable.signature.r,
                            redeemable.signature.vs
                        )

2. Recommendation

The recommendation is made to use abi.encode instead of abi.encodePacked to avoid any type of hash collision that may lead to greater loss.

[QYuQianchen]

It's correct that "abi.encodePacked can result in hash collisions when used with two dynamic arguments". However, I don't see how hash collision can happen particularly in the Channels.sol implementation. Please provide an example for _isWinningTicket

[Abbasjafri-syed]

unfortunately, currently not able to generate a collision as it requires significant resources like hashing power which is not available currently and the issue was highlighted as per solidity docs guideline.