Signatures can be reused across forks

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L244
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/abstract/EIP712.sol#L83-L89

Vulnerability details

Impact

In the case of a hard fork, the same signature can be considerd valid on both chains due to the lack on block.chainid computation in HolographERC20.permit().

Proof of Concept

The block.chainid will be used during initialization for HolographERC20.init() and EIP712_domainSeparatorV4().

https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L244

https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/abstract/EIP712.sol#L83-L89

In the event of a hard fork, approvals via permit on one chain will be considered valid on the other chain.

Recommended Mitigation Steps

Include the chainid on HolographERC20.permit() hash schema to prevent double spending and replay attacks in the case of a chainsplit/hard fork.

[gzeoneth]

nice to have hark fork replay protection but ./abstract is out-of-scope; will downgrade to qa

[alexanderattar]

Good find. Chain id should be included to prevent replays if there is a chain fork.

[ACC01ADE]

Further review of the code makes this an invalid issue since block.chainid is clearly used and checked in the EIP712.sol contract.
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/abstract/EIP712.sol#L88

[ACC01ADE]

Additionally, the cached data in EIP712 is updated if there is a change in blockchain chain id for some reason.
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/abstract/EIP712.sol#L76