code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Gravity.sol#L531-L560

Vulnerability details

Impact

Not including a chain ID in various hashes used throughout the system means that a matching transaction signed on one chain will be valid on another. The gravityId is only a potential mitigation, but the documentation on them is unclear. You may have 5 Gravity contracts on a testnest that use a unique gravityId, and 5 on mainnet with unique gravityIds, but the mainnet to testnet Id's match to make it easier for cosmos and nodes to test a closely to real life as possible.

Proof of Concept

An outlandish token transfer validated on a testnet gravity contract can be replayed by one compromised malicious validator.

Tools Used

Manual analysis

Recommended Mitigation Steps

Use chainId when generating signatures.

albertchon commented 2 years ago

Well obviously you wouldn't re-use the same validator set from a testnet on mainnet so this isn't really an issue in practice

Pedroais commented 2 years ago

As the sponsor said, this isn't really an issue. I think it should be invalid.

defsec commented 2 years ago

An attack vector is not clear, this is not possible in the real world scenario. the issue should be marked as an invalid.

code-423n4 / 2022-05-cudos-findings