MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.87k stars 4.84k forks source link

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

Open rmerom opened 7 years ago

rmerom commented 7 years ago

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.

danfinlay commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

danfinlay commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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

carver commented 7 years ago

@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 commented 7 years ago

@carver et al geth thread: https://github.com/ethereum/go-ethereum/pull/2940 our implementation: https://github.com/metamask/eth-sig-util usage examples: https://github.com/flyswatter/js-eth-personal-sign-examples

john-- commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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