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

Warn users to only add custom networks that they trust #8789

Merged
merged 3 commits into from
Jun 12, 2020

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented Jun 11, 2020

Adds a warning to the custom RPC form warning users to only add networks that they trust.
Copy is up for debate. Currently:

A malicious Ethereum network provider can lie about the state of the blockchain and record your network activity. Only add custom networks you trust.

Also gives the poor settings page sub-header some breathing room via top/bottom padding. It's only used in the custom RPC form.

Fixes #8667

Screenshot (old copy)

@rekmarks rekmarks requested a review from a team as a code owner June 11, 2020 21:59
@rekmarks
Copy link
Member Author

cc: @jakehaugen

@metamaskbot
Copy link
Collaborator

Builds ready [1d7ebf4]
Page Load Metrics (583 ± 16 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint28383232
domContentLoaded4456155823316
load4476165833316
domInteractive4456145813316

@rekmarks rekmarks force-pushed the custom-network-warning branch from 7d0cafb to 1d7ebf4 Compare June 11, 2020 22:33
@metamaskbot
Copy link
Collaborator

Builds ready [1d7ebf4]
Page Load Metrics (648 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint318641157
domContentLoaded3567596468038
load3587616488039
domInteractive3567596458038

@Gudahtt
Copy link
Member

Gudahtt commented Jun 12, 2020

I'm not sure about the copy; "record your on-chain activity" is decidedly innocuous, since on-chain activity is public anyway.

This goes beyond recording on-chain activity. The RPC endpoint would see all messages, many of which don't result in on-chain activity. It would also see your IP address, letting it correlate activity from different accounts sharing the same IP.

I'm not sure how to best word this, but those seem like the real threats to me - not on-chain activity. I'll see if I can think of an alternate wording.

@rekmarks
Copy link
Member Author

rekmarks commented Jun 12, 2020

@Gudahtt, yeah, it's a bit thorny because I doubt most users have any idea of the kinds of network activity going on.

How about "record your Ethereum-related activity"? Or "Ethereum-related network traffic"?

I was thinking about "read" instead of "record" also, but I'm concerned users might take it to imply that they can read private data.

Edit: Where we define "private" as e.g., your keys or something.

@Gudahtt
Copy link
Member

Gudahtt commented Jun 12, 2020

The RPC endpoint could also lie about the current state of the blockchain. That could be used to do sneaky things.

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@metamaskbot
Copy link
Collaborator

Builds ready [2103d50]
Page Load Metrics (697 ± 32 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint348844126
domContentLoaded5388536956732
load5408556976732
domInteractive5388526946732

@rekmarks rekmarks merged commit 5aabe2a into develop Jun 12, 2020
@rekmarks rekmarks deleted the custom-network-warning branch June 12, 2020 18:21
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

Successfully merging this pull request may close these issues.

Warn users to only add custom RPC endpoints that they trust
3 participants