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

Support replay protection via block.chainId #5

Open
sambacha opened this issue Aug 19, 2022 · 8 comments
Open

Support replay protection via block.chainId #5

sambacha opened this issue Aug 19, 2022 · 8 comments

Comments

@sambacha
Copy link

see: yieldprotocol/yield-utils-v2@13ec941

/// constructor 
deploymentChainId = block.chainid;
_DOMAIN_SEPARATOR = _calculateDomainSeparator(block.chainid);


/// ... Return the DOMAIN_SEPARATOR.
return block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR : _calculateDomainSeparator(block.chainid);


/// ...   IERC2612-permit abi.encode
block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR : _calculateDomainSeparator(block.chainid),
@merklejerk
Copy link
Owner

I don't understand the attack vector here. You sign a permit on a local fork with a different chainId and that gets out?

@sambacha
Copy link
Author

I don't understand the attack vector here. You sign a permit on a local fork with a different chainId and that gets out?

here is the thread, sorry! https://twitter.com/bantg/status/1560279527976161282

@merklejerk
Copy link
Owner

wow, that chain is already cursed 😬

@sambacha
Copy link
Author

wow, that chain is already cursed 😬

😂 it’s whatever you think ser, I can submit a PR if you would take it, I thought it best to see what your thoughts were before going any further. I plan on using this in production, so we may have this audited next month. Either way just thought it interesting at the very least!

Thanks again!

@merklejerk
Copy link
Owner

Yeah, let's do it.

@sambacha
Copy link
Author

sambacha commented Aug 22, 2022

Yeah, let's do it.

So have been thinking about this over the weekend, wdyt about a helper function to re-sign a message with a spending limit 0 to nullify the previous signature - both can be implemented to be sure, I think this may be helpful as there is no easily revoke permit like there is revoke approvals (eg revoke.cash)

edit: you may actually have to execute the permit function on the relevant erc20 contract and then execute a transaction to increment the nonce. That should verifiably nullify any messages signed before, as there may not be any unambiguous proof otherwise.

@merklejerk
Copy link
Owner

Why not just call increaseNonce()?

@sambacha
Copy link
Author

Why not just call increaseNonce()?

🪡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants