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

Remove Poseidon from PLUME [ERC 7524 incompatibility] #1174

Closed
Divide-By-0 opened this issue Oct 10, 2023 · 2 comments
Closed

Remove Poseidon from PLUME [ERC 7524 incompatibility] #1174

Divide-By-0 opened this issue Oct 10, 2023 · 2 comments
Labels
bug Something isn't working to-discuss Issues to be discussed further

Comments

@Divide-By-0
Copy link

Divide-By-0 commented Oct 10, 2023

We noticed that you deviated from ERC 7524 in your implementation of the PLUME nullifier, namely by using the Poseidon hash instead of the recommended hashes. We do not recommend that due to slowdowns in hardware wallets. It is possible that there is something in your implementation that I missed -- open to hearing from your cryptographers!

Edit: Earlier I had mentioned there may be wallet exploits that may be caused by repeated exponentiations of Poseidon hashes, but that is not the case as there is a map_to_curve happening that immunizes against this.

@Trivo25 Trivo25 added bug Something isn't working to-discuss Issues to be discussed further labels Oct 10, 2023
@Trivo25
Copy link
Member

Trivo25 commented Oct 11, 2023

Thanks for raising this issue! We talked this through in DMs, but just for the public records:

We use a map to curve algorithm which should allow us to prevent the issues mentioned in the blog post. By using this algorithm, we avoid the issue described below

Why do we not use a hash function like Poseidon? It turns out that Poseidon would actually lead to a critical vulnerability in ECDSA. Because we would know the pre-image of the final curve exponentiation, we would be able to generate a fake normal ECDSA signature for an arbitrary message. Basically, using the terminology of ECDSA Wikipedia, we would know a valid k corresponding to some r (r in this case would be hash(m, pk)), for which we would receive back a PLUME signature that we could use as the last term in the ECDSA signature, add any message hash(m) to, then multiply by k^−1, hence creating an arbitrary ECDSA signing oracle from just one PLUME signature – this would be quite bad. Thus, we must use a hash function for which the final step is not a curve exponentiation.

Please let us know if you have any other concerns with our implementation, we highly appreciate your input - this is super important to deliver a robust and secure SDK for writing zero knowledge applications!

@Trivo25 Trivo25 closed this as completed Oct 11, 2023
@Divide-By-0
Copy link
Author

Divide-By-0 commented Oct 11, 2023

@Trivo25 Continued discussion with Gregor leads me to conclude that yes, due to map_to_curve, your Poseidon hash to curve function is likely secure in the current implementation, because the final operation of your map to curve is not an exponentiation. However, it could be made faster both for calculation in hardware wallets and the client-side zk proof time.

Signature in Hardware Wallets

ZK Proof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working to-discuss Issues to be discussed further
Projects
None yet
Development

No branches or pull requests

2 participants