Security advisory: A malicious DAO update could reduce the registration duration of registered .eth 2LDs

DAO-Wide Temp Check
1

We were recently made aware of a security vulnerability in the .eth registrar controller, which would allow a malicious DAO update to appoint a registrar controller contract that shortens the duration of already-registered .eth names. This violates an intended invariant of the ENS system, which states that .eth registrations cannot be shortened once they are created.

The scope of this vulnerability is limited insofar as it would require DAO coordination to exploit it, and once implemented, the registration duration can only be reduced by 90 days per contract call. Still, this could be utilised to deliberately cause the targeted expiration of unwanted names.

The Ecosystem WG stewards have agreed to award a bounty of $100,000 from the bounty pool to the discoverer of this vulnerability.

The task before the DAO now, and which I would like input on, is whether and how we should mitigate this vulnerability. We have published a security advisory on GitHub detailing the vulnerabilty and including code that will eliminate it.

The mitigation code works by appointing a new ā€˜adminā€™ contract as the owner of the .eth registrar and the name wrapper, which passes through most admin commands. New registrar controllers, however, will have to be appointed via a dynamically created proxy contract that properly enforces the invariant that registration durations will not be reduced.

The consequence of implementing this mitigation is that all registrations and renewals that take place on new controllers deployed after the mitigation will consume additional gas - on the order of an additional 3250 gas. It also adds some complexity to the code path and deployment process of ENS. Registrations via existing controllers will not be affected, as they are known safe and can be grandfathered in.

A similar vulnerability has been discovered in the name wrapper, which would permit the DAO to appoint a malicious upgrade contract; if approved the upgrade contract could seize control of wrapped names, even if the user does not choose to upgrade the name. The mitigation contracts also mitigate this vulnerability.

I would welcome insight from the DAO as to whether we consider the cost of patching this vulnerability worthwhile, and if so whether the approach taken in the proposed patch is the correct one.

4 Likes
2

My vote is on making the necessary code changes to resolve this vulnerability.

I donā€™t expect that vulnerability to ever be exploited as it would require a full DAO vote to approve a malicious controller. However, the optics and peace of mind is well worth the extra 3250 gas (which is only like a 1% increase from the 250,000-400,000 gas the register transaction currently consumes).

2 Likes
3

Itā€™s a very clever finding, but hard to exploit in my opinion. For those who didnā€™t read or understand the technical text, itā€™s an overflow attack on the renewal, meaning that if you renew for someone elseā€™s name for a near infinite amount of time (2^256 seconds) then its expiration date can be moved to the past. Overflows are a common attack and the code has a guard against it, but the exploiter discovered that the check fails to safeguard it if the duration is precisely ā€œinfinite minus 90 daysā€ which is honestly quite a funny concept, meaning the attacker can move the registration back for 90 days.

BUT the attack isnā€™t exploitable right now, because registering that would also require a near infinite amount of ether. So it requires a second level of attack in which someone must either find a vulnerability in the registrar that would allow someone to register names for free, or the attacker would need to create a social attack by somehow being able to approve a new backdoored registrar.

I would suggest the following course of action:

  1. Awareness: now that weā€™re aware of this, any update to the registrar, or any proposed mechanism that would allow names to be renewed for free should have extra scrutiny.

  2. Grace Period: we should treat the 90 days grace period like the hidden volume of extra gas you have on your car, once it says your fuel is depleted. You should never allow someone to reach that point, but if they do theyā€™ll be happy it exists. There are many reasons not to treat the grace period as a ā€œfreebieā€ registration duration and this one is one more

  3. Pack it on the next system upgrade. .eth controller canā€™t be upgraded but there are other pieces that can. I donā€™t see any reason to make a system upgrade for this issue, but we should simply have it ready for whenever we have enough features to merit a new contract upgrade. Maybe weā€™ll have other gas saving techniques that will outweigh the extra expense that this new owner model will have. The other mitigations should be enough.

This is of course just my opinion, which I am willing to change.

11 Likes
4

Great points here.

If I understand everything correctly here, the attack would require a DAO vote to approve a malicious .eth registrar controller, where the malicious .eth registrar controller would include a manipulation in the rentPrice for any / all names targeted for attack, thereby bypassing the need for a near infinite amount of ether.

1 Like
5

I think itā€™s more direct than that. The new controller could just call the renew function directly and submit whatever value it wanted for the duration of any name. The DAO always has the power to extend the expiry of any name at any time, but it shouldnā€™t be allowed to reduce the expiry of a name.

2 Likes
6

The attack is repeatable, so itā€™s not just the grace period that is at risk. The cost is four transactions/year to execute, which could also be batched in order to reduce the cost.

The fix will most likely be to wrap the registrar with a new owner/controller that can check to make sure there are no overflowing values submitted to the renew function. This is a form of upgrading that can only limit the input values, but canā€™t really change the base functionality of the registrar. For this reason I donā€™t see this being a part of any future upgrades, and this would be a one time fix.

1 Like
7

Thatā€™s right; see this change.

1 Like
8

Ah of course good point. You could potentially batch enough transactions that a high value name renewed for years now has its expiration date so much in the past enough to allow you to register it without any premium. And then you could batch more transactions to commit to register it for yourself in the same transaction.

1 Like
9

This exploit strikes at the core of one of the primary promises of a .eth name. The promise that once you register a name it will be yours to own for as long as you want to register it; That the privilege of name ownership shall not be infringed.

Fortunately, executing this exploit requires a malicious DAO to actually vote to use it, reducing its immediate threat.

We should fix this exploit, but Iā€™m also comfortable taking a ā€œmeasure twice, cut onceā€ approach to the resolution. This means Iā€™d like us to have a valuable dialogue not only about the fix but also about its timing. I support the suggestion from the metagov call to hold a social vote on this during the next voting window. This will ensure we get everyoneā€™s eyes on it and truly act with the will of the delegates.

The likelihood of a malicious DAO or a malicious executable proposal passing currently is low. This makes me suggest a bit of patience with our response to ensure we get it right.

Thanks for this. Is this still WIP or would you call it ready for review?

1 Like
10

I think this is a bug that needs to be fixed, but not a ā€œmust nowā€ fix. Instead, I think the time before fixing it is very meaningful:

  1. This time was a test for ENSDAO as an organic organization. I personally donā€™t think such an attack will happen (or succeed) because in some extent the biggest victim of the attack is the person or organization capable of launching it.

  2. This time can be used to continue to discover the features that the controller contract needs to improve. It is not worth immediately changing the contract and adding complexity to the system just because of an unforced vulnerability.

3 Likes
11

Weighing in myself after first listening to feedback from the ecosystem. I helped formulate and review the PR with Nick and I think this is probably the best path that trades off a small gas increment for all users, but protects the vision and values of ENS as a protocol, and .eth as a namespace.

I absolutely agree on the measure twice, cut once approach and would be open to hearing other possible technical solutions that myself or Nick havenā€™t thought of yet.

3 Likes
12

Itā€™s ready for review.

1 Like
13

Cross posting my comment from [Temp Check] Make Wrapped Names Immutable - #6 by MicahZoltu because I think it is important and many may be subscribed to this issue but not see that one.

TL;DR: I really think both of these issues should be fixed now and not delayed until ENSv2.

14

The only avenue for fixing the main vulnerability disclosed in this post is the wrapper approach, but given that registrations on v1 will be stopped as part of the migration to v2, thereā€™s little point in implementing that: better for the DAO to simply revoke control over the v1 registrar once migration is begun.

15

My concern with this approach is that ENSv2 is still an undefined number of years away, and this is an issue affecting ENSv1 today. We can hope that no one attacks ENS, but I donā€™t think we should rely on that when we have a viable solution to the problem that can be deployed quickly.