Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replay attack for deployHolographableContract() #178

Closed
code423n4 opened this issue Oct 24, 2022 · 3 comments
Closed

Replay attack for deployHolographableContract() #178

code423n4 opened this issue Oct 24, 2022 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) edited-by-warden invalid This doesn't seem right resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) responded The Holograph team has reviewed and responded sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/HolographFactory.sol#L192

Vulnerability details

Impact

Detailed description of the impact of this finding.
Function deployHolographableContract() is subject to reply attack since the signature does not sign chainid and address of the contract HolographFactory. As a result, the same signature can be replayed at another chain with another HolographFactory, leading to creating the contract to be created either before schedule or in the chain that is not planned.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/HolographFactory.sol#L192

Tools Used

Manual

Recommended Mitigation Steps

To avoid reply attack, the signature must sign the chainid and the address of the HolographFactory contract as well.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Oct 24, 2022
code423n4 added a commit that referenced this issue Oct 24, 2022
@code423n4 code423n4 changed the title Replay attack for Replay attack for deployHolographableContract() Oct 24, 2022
@gzeoneth
Copy link
Member

Related to #261 from the same warden, there actually is a chainType but not validated onchain.

@gzeoneth gzeoneth added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Oct 31, 2022
@gzeoneth
Copy link
Member

Although I don't see any problem for it to be replayed, the owner will still be the same EOA.

@gzeoneth gzeoneth added disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) and removed sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons labels Oct 31, 2022
@alexanderattar
Copy link

Design decision. Deployment signature has chainId intentionally left out to be able to replay on other chains which is one of the core premises of the protocol

@alexanderattar alexanderattar added sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue responded The Holograph team has reviewed and responded labels Nov 8, 2022
@alexanderattar alexanderattar added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Nov 15, 2022
@gzeoneth gzeoneth added the invalid This doesn't seem right label Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) edited-by-warden invalid This doesn't seem right resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) responded The Holograph team has reviewed and responded sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

3 participants