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

untyped data signing #447

Closed
Tracked by #88
code423n4 opened this issue Oct 25, 2022 · 1 comment
Closed
Tracked by #88

untyped data signing #447

code423n4 opened this issue Oct 25, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working invalid This doesn't seem right 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/main/src/HolographFactory.sol#L107-L116

Vulnerability details

in function deployHolographableContract the bytes32 hash is directly encoded without adding any domain separator .
this will cause several issues
an attacker can front run the signature and use them on same contract on another chain . eg a user wants to call deployHolographableContract in polygon network . an attacker use same signature and make the same transaction in another network that holograph exists .
an attacker can front run the signature from different holograph 's contracts . let's imagine there is a HolographFactory.sol that was containing a vulnerability so another contract will be made using same source code that does not contain the issue . A user callls deployHolographableContract and interact with the fixed one contract . attacker sees the tx and execute it using same signature on the contract that had vulnerability .
an attacker can front run the signature from another project that had same hash type and use it to execute deployHolographableContract .

recommendation :
1.) There is always a domain separator that includes the contract address.
2.) The chain ID is included in the domain separator
3.) There is a type hash (of the function name / parameters)
4.) The domain separator does not allow reuse across different projects, phishing with an innocent

https://eips.ethereum.org/EIPS/eip-712

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 25, 2022
code423n4 added a commit that referenced this issue Oct 25, 2022
@gzeoneth
Copy link
Member

@gzeoneth gzeoneth added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Oct 31, 2022
@alexanderattar alexanderattar added the responded The Holograph team has reviewed and responded label Nov 8, 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
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working invalid This doesn't seem right 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