BuidlGuidl / SpeedRunEthereum

speedrunethereum.com 🏃‍♀️
https://speedrunethereum.com
MIT License
125 stars 81 forks source link

Change message to sign #30

Open dgrcode opened 3 years ago

dgrcode commented 3 years ago

This is not an MVP feature, but something I think we should consider.

I see the message to be signed something like a CSRF token. If we always send the same message, the same addresses will always produce the same signature, so an eavesdropper will be able to re-authenticate maliciously with the signature they found.

Why we shouldn't worry too much

I don't think this is a big deal because:

Why I think this should be implemented at some point

However, I think changing the message to be signed shouldn't be too hard. The only concern here is storing the message to compare it with the signed message. For that, an in-memory dictionary with { [address]: messageToBeSigned } should work completely fine. At least until we have >10k users signing in at the same time (should be fine).

(cc @carletex @sogasg)

dgrcode commented 3 years ago

From a comment in #48

Nothing to change here, but something to keep in mind. Once we have non-deterministic input to generate the messages (like nonce, timestamp, etc), we'll need a way to store the message even if just in memory because it won't be possible to regenerate the message as we're doing in (currently) line 30:

const trustedMessage = getSignMessageForId(verifyOptions.messageId, verifyOptions);

We also need to update all the messages to sign, not just the one at /sign (we only had that one when the issue was created)