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

Potential vulnerability with web3.personal.sign as authentication mechanism #1839

Open
rmerom opened this issue Jul 27, 2017 · 18 comments
Open

Comments

@rmerom
Copy link

rmerom commented Jul 27, 2017

Problem:
Suppose mysensitiveinfo.com uses web3.personal.sign for authentication.
malicious.com is another website that lures users into authenticating. When a user asks for a challenge message for her to sign for address A, malicious.com's server requests a similar challenge for A from mysensitiveinfo.com . malicious.com then relays the challenge to the user, gets the message signed and can now log into mysensitiveinfo.com .

Possible mitigation:
Have MetaMask emulate the same-origin policy by requiring messages that are to be signed to be prefixed with the page's domain name + some delimiter.

@rmerom rmerom changed the title Possible vulnerability with web3.personal.sign as authentication mechanism Potential vulnerability with web3.personal.sign as authentication mechanism Jul 27, 2017
@danfinlay
Copy link
Contributor

That's an interesting vector, thanks for bringing it up.

Sites could help mitigate this by including obviously site-specific information in their challenge text, but we could probably also bake some security into the API here.

We could potentially propose an expanded personal.sign protocol where the data to sign follows a certain format, and can specify a domain that it's valid on.

If this domain field is recognized, MetaMask could refuse to sign it on another domain.

Requires expanding the personal.sign data protocol, which we've wanted to do. There was an interesting proposal to use email-like headers in the sign payload, allowing specifying multiple rendering formats, for example, and any other metadata like this. #925

@rmerom
Copy link
Author

rmerom commented Jul 27, 2017

Yes, both sound like good ideas - the email headers, as well as the website adding site-specific information. I'll be happy to join the conversation on the first if one exists.

However, I'm wondering if, for the time being, you think that MM should display any warning when using web3.personal.sign? although it's not as risky as web3.eth.sign, it can still pose a MITM attack risk for users without them knowing.

@danfinlay
Copy link
Contributor

That sounds reasonable. I'm juggling a lot of things before I'm out for a couple weeks, if anyone wants to propose wording for the wording here, that could speed this along.

@kumavis
Copy link
Member

kumavis commented Aug 2, 2017

thanks for filing the issue. we're following the web3.personsal.sign spec correctly here.

We could potentially propose an expanded personal.sign protocol where the data to sign follows a certain format, and can specify a domain that it's valid on.

I think creating a new domain auth spec as dan proposed is the correct thing to do

@chejazi
Copy link

chejazi commented Aug 8, 2017

Seems like the server for mysensitiveinfo.com should manage the responsibility of generating unique messages (e.g. uuids) for the user to sign. Imposing a site-specific format introduces complexity in using signatures on the blockchain. For instance, imagine a scenario where you want to sign something like a content hash, and to submit that signature to a smart contract. All contracts would need to understand the site-specific policy in order to reason about a content hash and a signature.

@rmerom
Copy link
Author

rmerom commented Aug 8, 2017

@chejazi can you elaborate on how mysensitiveinfo.com could avoid MITM attacks given the attack vector above?

@danfinlay
Copy link
Contributor

Need to distinguish between:

  1. Signatures to be used by a smart contract
  2. Signatures to be used by a site for authentication.

Both should be responsible for their own anti-replay logic, like nonces, utxos, or ttls.

Yes it would be more complicated for an anti replay policy that worked on both contracts and offchain, this seems like a problem for a straw man to solve, we could add real security with a domain-locking format/policy.

@chejazi
Copy link

chejazi commented Aug 8, 2017

I might be wrong on this, but AFAIK there is no need for a replay protection for message signing (they're not transactions, they don't increment the nonce, etc).

@rmerom I would deliver the message to sign over https so there is no possibility to MITM. And to avoid the potential for collisions I would use something like uuids.

@rmerom
Copy link
Author

rmerom commented Aug 8, 2017

@FlySwatter +1
@chejazi assume I'm malicious.com . When a user tries to sign into my page using public address A, I send a login request for A to mysensitiveinfo.com . mysensitiveinfo.com might use any format/uuid they like, but I simply convey it back to my user. Once they sign the message, I send the signed message to mysensitive.com and I got logged in! I'm not sure how we can prevent this with https or uuids?

@chejazi
Copy link

chejazi commented Aug 9, 2017

mysensitiveinfo.com could prevent that with proper CORS policy, e.g. using an "Access-Control-Allow-Origin: mysensitiveinfo.com" header in it's responses. That way, a login message (uuid, etc) could only be generated by visiting mysensitive.com directly.

@rmerom
Copy link
Author

rmerom commented Aug 10, 2017

@chejazi I think we had a misunderstanding - I'm not suggesting malicious.com would embed/ajax a call to mysensitiveinfo.com . I suggest it'll contact mysensitive.com from its server, then relay the message to sign to the user of malicious.com .

@chejazi
Copy link

chejazi commented Aug 10, 2017

That could be solved with sessions. Say my browser has a cookie session with mysensitiveinfo.com. Stored in that session data (server side) is the message it gives to the user to sign. Then when the user submits the signed message, it can validate that the message is part of that user's session (and there is no threat of spoofing around this).

So even if malicious.com figured out the message generated for a user on mysensitiveinfo.com and got the user to sign it, it wouldn't matter because any sessions created from malicious.com's servers to mysensitiveinfo.com's servers would have different messages.

@chejazi
Copy link

chejazi commented Aug 10, 2017

NVM! I see your point. Please disregard previous message. Thanks for being patient :)

@carver
Copy link

carver commented Sep 14, 2017

@kumavis which web3.personal.sign spec are you referring to? I don't see it mentioned here: https://github.com/ethereum/wiki/wiki/JavaScript-API

@kumavis
Copy link
Member

kumavis commented Sep 14, 2017

@john--
Copy link

john-- commented Nov 9, 2021

Hi. It's been a few years since this was brought up. Is there a newer spec that incorporates domain which solves this? Or is the solution to embed session related info into the request?

@carver
Copy link

carver commented Nov 9, 2021

Is there a newer spec that incorporates domain which solves this?

Yes, but it's not backwards compatible. Both version 0 and 1 of EIP-191 include components that could be used to validate who generated the message for signing. Note that personal_sign is "version 0x45" of the EIP-191 standard, despite being the oldest.

For this thread's reasons, and other incompatibility issues (like the awful way that 0x45 encodes message length, and how the different implementers treat unicode characters in messages differently), I'd argue that personal_sign should be removed completely, as quickly as can be tolerated.

@vandan
Copy link

vandan commented Jul 21, 2022

If the Sign-in with Ethereum pull request is merged, then it should provide an improvement for cross-domain signing.

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

No branches or pull requests

10 participants