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

Tweaks and creaks: Disable UNS, fix Ledger signing on built-in non-ETH/MATIC networks #3746

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

Shadowfiend
Copy link
Contributor

@Shadowfiend Shadowfiend commented Jul 15, 2024

These are unrelated changes: UNS is disabled due to compromise during the late-last-week SquareSpace domain theft issues, while the Ledger signing issue is long-standing. The Ledger signing issue is partial—it doesn't work for custom networks, and it is handled in a way that's different for EIP191 messages than for other messages; further refinement is needed to fix it the rest of the way.

Latest build: extension-builds-3746 (as of Mon, 15 Jul 2024 21:10:18 GMT).

UNS was compromised in the SquareSpace domain hack triggered by the
Google Domains shutdown. They've turned off services but DNS hijacking
means we will avoid trusting until things are back to normal, at which
point an updated API key will be issued and UNS will be re-enabled with
the updated key.
Ledger was checking specifically for those two networks from a constant
that was never updated. We switch instead to check whether the network
in question shares a derivation path with the one used by the Eth app
(aka the Ethereum derivation path).

Custom networks added by users currently don't set a derivation path, so
they will still fail this test (as well as the guards on signing
messages and typed messages, which are already using derivation paths).
The Ledger-specific signing screen showed the domain and message hashes
with a startin 0X due to an aggressive toUpperCase call. To make it
easier to scan, it now starts with 0x (lowercase x) as expected.
@Shadowfiend Shadowfiend merged commit fa852d4 into main Jul 15, 2024
5 of 6 checks passed
@Shadowfiend Shadowfiend deleted the tweaks-and-creaks branch July 15, 2024 20:47
@Shadowfiend Shadowfiend mentioned this pull request Jul 15, 2024
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.

1 participant