ArkProject: NFT Bridge

Summary

The StarklaneMessaging contract is vulnerable to hash collisions due to the casting of uint256 values to bytes32. This issue can lead to incorrect state management within the _autoWithdrawn mapping, affecting the contract's ability to accurately track message statuses for auto-withdrawal.

Vulnerability Details

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Messaging.sol#L46

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Messaging.sol#L34

In the method addMessageHashForAutoWithdraw(), a uint256 input is cast to a bytes32. If the higher-order bits of a uint256 exceed 32 bytes, they are truncated, resulting in only the lower 256 bits being considered. This can cause distinct uint256 values to map to the same bytes32, leading to hash collisions. The _autoWithdrawn mapping relies on unique hashes for accurate state tracking using an enum-like pattern (WITHDRAW_AUTO_NONE, WITHDRAW_AUTO_READY, and WITHDRAW_AUTO_CONSUMED). Collisions can disrupt this tracking.

Impact

Hash collisions could cause one message's status in _autoWithdrawn to overwrite another's, leading to incorrect state management.

Messages might be processed incorrectly, resulting in unauthorized withdrawals or blocked legitimate transactions.

An attacker could exploit this vulnerability by crafting specific values that collide with existing hashes, potentially altering withdrawal processes.

The contract logic that differentiates between auto and regular messaging could be compromised if messages are misidentified due to hash collisions.

Poc:

Test for hash collisions due to casting a uint256 to bytes32. designed to check if different uint256 values result in the same bytes32 hash when truncated.

logs after running testHashCollision

Analysis of the Logs:

1. First Hash:

   • 0x00000000000000000000000000000000ffffffffffffffffffffffffffffffff

   • This bytes32 hash is derived from a uint256 value where the higher-order bits are zero.

2. Second Hash:

   • 0x11111111111111111111111111111111ffffffffffffffffffffffffffffffff

   • This bytes32 hash is derived from a uint256 value where the higher-order bits are non-zero, but the lower 256 bits (the part that remains in bytes32) are identical to the first hash.

Collision Demonstrated: The logs show that the two uint256 values, when truncated to bytes32, produce the same lower 256 bits. This demonstrates a hash collision due to truncation.

The crafted values successfully illustrate that different uint256 values can result in the same bytes32 hash when only the lower 256 bits are considered.

Tools Used

Manual review

Recommendations

1. Change Input Type: Use bytes32 directly for inputs instead of casting from uint256.

2. Incorporate a Check:

   • If retaining a uint256, add a check before casting:

     require(msgHash <= type(uint256).max >> 96, "msgHash exceeds 32 bytes");

     ​

3. Use Hashing Functions: Consider using functions like keccak256 for consistent and collision-resistant hashes.