ginlee - Two different transactions can result in the same transactionHash

[sherlock-admin]

ginlee

medium

Two different transactions can result in the same transactionHash

Summary

Vulnerability Detail

  bytes32 transactionHash = IReality(_msgSender()).getTransactionHash(     
                to,  
                value,
                data,
                operation,
                nonces[i]
            );

As we can see in getTransactionHash, lack of chainId may lead to same transactionHash is generated for different chains

Impact

If there is no chainId to distinguish between different chains, then two completely different transactions on different chains might generate the same hash. This means that the function could mistakenly consider a legitimate transaction as having been vetoed because it shares the same hash with a transaction from another chain

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/zodiac/core/SafeGuard.sol#L63-L69

Tool used

Manual Review

Recommendation

It is recommended to include the ChainId in the getTransactionHash to calculate the transactionHash. By doing so different transactions coming from different chains will not result into the same transactionHash

[amshirif]

The components that make up the hash are not in our control. We are preparing the hash in a way to integrate with a different set of contracts.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { This is invalid is thje watson did not show in practice how that same hash can be generated}

[nevillehuang]

Invalid, agree with sponsor, no plans to propose transactions in other chains yet anyways