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

ENS Normalization Update Needed #7095

Open
lightwalker-eth opened this issue Dec 12, 2023 · 4 comments
Open

ENS Normalization Update Needed #7095

lightwalker-eth opened this issue Dec 12, 2023 · 4 comments

Comments

@lightwalker-eth
Copy link

lightwalker-eth commented Dec 12, 2023

Problem Definition

  1. Rotki is not supporting the latest ENS Normalize standard (ENSIP-15).
  2. Rotki is displaying reverse lookup ENS names (primary ENS names) to its users that are not already in ENS Normalized form.

Background

Rotki has a dependency to web3.py version 5.31.3 - released Dec 2, 2022. This version of web3.py implements the old ENSIP-1 normalization algorithm.

ENS approved a new normalization standard, ENSIP-15, on June 15, 2023.

Best practices for displaying ENS primary names include verifying that the primary name is already ENS normalized. If a primary name is not already in ENS normalized form an application should act as if that address has no primary name defined at all.

Example Use Case

0xfA9A134f997b3d48e122d043E12d04E909b11073

Not the real 888.eth

This address appears in Rotki as "888.eth". However this is deceiving. There are invisible characters in this name. This is not the true "888.eth". For details, check the NameGuard report for this name. You can also check the raw JSON output of NameGuard's Secure Primary Lookup result for this address.

Potential Impacts

There are a number of potential impacts. One example:

Someone could potentially be exploited into sending funds to the wrong address. For example, a Rotki user might see that they recently received funds from "vitalik.eth" and then use Rotki's UI to copy the address of that account to send a follow up transaction back. However this might not be the real "vitalik.eth". The funds could be send to an unintended address.

Suggested fix

Consider making use of ENS Normalize Python. This library is built and actively maintained by the team at NameHash Labs as part of the NameGuard solution that helps keep the ENS community safe from hidden risks or limitations in ENS names.

Our team actively maintains this library to ensure it is always in full compliance with the latest ENS standards. We have 100% test coverage across major operating systems.

Alternatively you could update the web3.py dependency to the newest version (at least v6.6.0 2023-07-12, which added an implementation of ENSIP-15). However, the web3.py implementation of ENS normalization algorithm is not up to date with recent changes to the ENS Normalize Standard (e.g. supporting emojis introduced in Unicode 15.1.0), has some minor flaws, and is slower.

System Description

Applies to the latest version of Rotki (1.13.1 as of this report) running on any Operating System.

@lightwalker-eth
Copy link
Author

For additional background on how it is possible for ENS names with invisible characters to exist (or other nasty cases) this post on the ENS DAO forum might help: https://discuss.ens.domains/t/underscores-are-allowed/18397/8?u=lightwalker.eth

@lightwalker-eth
Copy link
Author

We're also happy to assist with a PR if you would like to explore the use of ENS Normalize Python.

@LefterisJP
Copy link
Member

Thank you for the report @lightwalker-eth.

We will see what we can do. Upgrading web3 is probably the easiest way to go about it though, it can be hard due to some internal functions we use. It's one of these upgrades which may take more time than we want.

Could also try a specific library for this, but not sure how much dev time this would need and our priorities right now are for other things.

@lightwalker-eth
Copy link
Author

Sure appreciate everything there @LefterisJP.

We'll continue making efforts to support web3.py with ENS normalization. Perhaps we'll be able to help them soon. For now though even their latest implementation is out of date.

We're happy to look into what might be needed for a Rotki PR. Perhaps one of our senior engineers @djstrong could explore. If we're lucky it's small, but good to have a more detailed investigation first. Just let us know if you'd like us to have a more detailed look 👍

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

No branches or pull requests

2 participants