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

Improve warning text for eth_sign #11337

Closed
4 tasks done
danfinlay opened this issue Jun 17, 2021 · 8 comments
Closed
4 tasks done

Improve warning text for eth_sign #11337

danfinlay opened this issue Jun 17, 2021 · 8 comments

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Jun 17, 2021

I know, some people would have us simply remove support for eth_sign. I'll leave that debate out of this issue.

While we support it, we should ensure its warning text is as useful as possible. The current text is correct, but also diluted with multiple messages, making it less impactful than it could be.

image

In an EIP 3074 environment, a blind signature can sign over total control of an account with even greater certainty.

  • We do fortunately already disclaim this signature could hand over control of the account
  • If EIP 3074 approaches, we should probably add an additional, even more grave warning, with extra friction, for any signature with an EIP 3074 0x03 prefix.

Multiple phases we could do here.

  • Improve this warning text
  • Add some padding to the sides of this text.
  • Move this confirmation to our new more consistent confirmation format.
  • Add more friction to this confirmation ("are you sure? This can give full control of your account to this site.")

This is basically a very advanced method that we should imagine "nobody but expert devs who really know what they're doing" should be using these days. You could consider this a step in the direction of deprection, but rather than breaking dapps that are relying on it, we're just making this antipattern more painfully visible to users.

Proposed copy

We are unable to estimate the result of this signature. This signature could potentially perform any operation on your account's behalf, including granting complete control of your account and all of its assets to the requesting site. Only sign this message if you know what you're doing or completely trust the requesting site.

@sambacha
Copy link

sambacha commented Sep 29, 2021

Can you at least add it to your documentation that it is still supported? https://metamask.github.io/api-playground/api-documentation/

Also, for the proposed copy, a more accurate verbiage would be:

We are unable to provide a human readable output of this signature. 
This signature could potentially perform any operation on your account's behalf, including granting complete control of your account and all of its assets to the requesting site. Only sign this message if you know what you're doing or completely trust the requesting site.

@Gudahtt Gudahtt added type-enhancement and removed Sev2-normal Normal severity; minor loss of service or inconvenience. area-copy labels Nov 9, 2021
@UXDesignFine
Copy link

I created a design mock up with a new warning message. I think the warning is much clearer to the average user. Let me know what you all think. Thanks!

metamask-signature-warning

@sambacha
Copy link

sambacha commented Sep 6, 2022

I created a design mock up with a new warning message. I think the warning is much clearer to the average user. Let me know what you all think. Thanks!

metamask-signature-warning

Bro this design is beyond useless its misleading.

@UXDesignFine
Copy link

UXDesignFine commented Sep 9, 2022

@sambacha

I disagree, signature requests have been contributing to many recent thefts. The problem is that the current warning is not clear enough that signing a request can lead to a theft of a users assets. As it is designed, it is easy to overlook the warning.

The current message is:

"Signing this message can be dangerous. This signature could potentially perform any operation on your account's behalf, including granting complete control of your account and all of its assets to the requesting site. Only sign this message if you know what you're doing or completely trust the requesting site."

The message is too long and the way it is written is confusing.

My version is:

"WARNING: Only sign this request if you completely trust the requesting site. Signing this request could potentially allow the requesting site to take complete control of your account and all of its assets."

I believe my version will help warn users of the risks.

@sambacha
Copy link

sambacha commented Sep 9, 2022

@sambacha

I disagree, signature requests have been contributing to many recent thefts. The problem is that the current warning is not clear enough that signing a request can lead to a theft of a users assets. As it is designed, it is easy to overlook the warning.

The current message is:

"Signing this message can be dangerous. This signature could potentially perform any operation on your account's behalf, including granting complete control of your account and all of its assets to the requesting site. Only sign this message if you know what you're doing or completely trust the requesting site."

The message is too long and the way it is written is confusing.

My version is:

"WARNING: Only sign this request if you completely trust the requesting site. Signing this request could potentially allow the requesting site to take complete control of your account and all of its assets."

I believe my version will help warn users of the risks.

Your solution does not solve existing dapps who rely on the method - without first solving the needed use of that method people will ignore it because people are using it for legit reasons. Ergo, it does not actually solve anything.

@UXDesignFine
Copy link

@sambacha

Yes, the ideal solution would be to verify what the signature request will be allowed to perform so the warning messages could be specifically tailored to the risks. Whether this will be possible in the future, I don't know.

My wording is not going to solve that, but I do believe my wording will help make the risks clear to less sophisticated and non-native English speaking users.

By highlighting the text in red, will also help bring attention to the warning, as many users have been ignoring the message as it is currently designed.

Theft of assets by signature requests are becoming a big problem. I think some small changes here could make a big difference.

UXDesignFine added a commit to UXDesignFine/metamask-extension that referenced this issue Sep 9, 2022
Updated Signature Request Warning Message for MetaMask#11337

Theft of assets by signature requests are becoming a big problem. I believe my wording to the signature request warning will help make the risks clear to less sophisticated and non-native English speaking users and help prevent thefts from signature requests.
@danjm danjm unassigned ryanml Nov 1, 2022
@brad-decker brad-decker reopened this Nov 16, 2022
@brad-decker
Copy link
Contributor

The portion of this that includes updating test copy was done in #12034

@holantonela
Copy link

I must consider this ticket closed after we introduce #15932.

Also, we are disabling eth_sign by default making it optional for users to recieve eth_sign messages.

Please, re-open if you consider there is a remaining work on it.

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

7 participants