Unwrapped ENS names can be made immutable.
No one and no group can steal a name from someone. The ENS DAO can prevent renewals, but they cannot transfer a name away from a user or edit the records of that name. If the user transfers the name to a burner address, the name is functionally immutable until it expires.
Current situation for wrapped names
Wrapped ENS names cannot be made immutable.
ENS DAO has the ability to steal any wrapped name from any user, regardless of fuses burnt or ownership being transferred to a burner address. There is no way for a user to revoke this power for a wrapped name once the CANNOT_UNWRAP fuse is burnt other than by the ENS DAO “stealing” the name and giving it back to the user in unwrapped form.
Proposed Change
Remove ENS DAO’s ability to steal wrapped names
ENS DAO should not have the ability to take names from people, and a name that has the appropriate fuses/ownership burnt should be truly immutable until expiry.
Technical options for resolution
There are two reasonable options to resolve this that I can see from a cursory look at the NameWrapper contract.
Revoke ownership of NameWrapper. This would take away this ability, but it would have the side effect of removing the ability of the DAO to call the setMetadataService function. I’m not sure what that does, but it may be necessary to retain power over.
Transfer ownership of NameWrapper to a contract that can only call the setMetadataService function. The current TimelockController owner would become the owner of this new intermediate contract.
Technical details of how The DAO can steal a name from an individual
The NameWrapper has the following function on it:
/**
* @notice Set the address of the upgradeContract of the contract. only admin can do this
* @dev The default value of upgradeContract is the 0 address. Use the 0 address at any time to make the contract not upgradable.
* @param _upgradeAddress address of an upgraded contract
*/
function setUpgradeContract(INameWrapperUpgrade _upgradeAddress) public onlyOwner {
if (address(upgradeContract) != address(0)) {
registrar.setApprovalForAll(address(upgradeContract), false);
ens.setApprovalForAll(address(upgradeContract), false);
}
upgradeContract = _upgradeAddress;
if (address(upgradeContract) != address(0)) {
registrar.setApprovalForAll(address(upgradeContract), true);
ens.setApprovalForAll(address(upgradeContract), true);
}
}
This function can only be called by the NameWrapper owner, which is a TimeLockController presumably owned by the ENS DAO (it is difficult to verify the actual owners of the TimeLockController as they are stored in a mapping, so one needs to write a script that searches all event logs to find current ownership set).
The TimeLockController (e.g., The DAO) can call that method and provide any address (EOA or contract) as the upgradeAddress. That address will then gain the ability to transfer any name away from the NameWrapper and give it to any address they choose.
This was not intended behaviour, and we should fix it - particularly given that we’re not likely to use the upgrade mechanism; the v2 migration will instead rely on users transferring their names to a migration contract.
Rather than either of the proposed solutions, I’d suggest deploying a new metadata contract using a proxy, updating the wrapper to point to that, and then revoking ownership of the NameWrapper. The ability to set metadata is the only admin functionality on the namewrapper besides setting an upgrade target, and if we use a proxy here, we can revoke ownership of the name wrapper without the need for an intermediate contract.
What are the next steps for getting this addressed? Is this something the ENS dev team can/will take on and fix without further involvement from me or the DAO (other than voting to deploy it at some point)?
For reference, this is the post where the vulnerability was first announced. Several measures to fix this bug have been discussed, and I expect that, along with the final ENSv2 plans, a fix will be implemented.
I’m assuming you are referring to this footnote (which isn’t mentioned in the security advisory)?
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.
At the least, a security advisory should be created that includes this.
I would like to advocate that fixing this does not wait for ENSv2. My team has an ENS app that we built and are waiting for a fix to this before we deploy. Our app (which allows users to easily create/extend immutable subnames) was built under the assumption that wrapped ENS names were actually immutable (up to expiration), but at the moment this is not the case due to this issue and we don’t want to endorse a product and make claims about “immutable subnames” when they aren’t actually immutable (just protected by the good grace of the DAO, not terribly different from DNS names).
I feel the same way about the other bug mentioned in that link, it really feels like it should be fixed without waiting for ENSv2 as at the moment ENS is touted as a censorship resistant tool, but one can easily imagine state pressure on ENS DAO token eholders, a hostile takeover of the DAO, or hacked ENS DAO voters causing that censorship resistance guarantee to be compromised.
Delaying this to be fixed in “v2” also assumes that all users will want to move to v2. As I have mentioned in the v2 discussion thread, I already pretty strongly disagree with the pressure being applied to the ecosystem to migrate to v2, and leaving bugs unfixed in v1 feels like a pretty underhanded way to force people to use ENSv2 even if they don’t like it. For this reason alone, I think this should be fixed before ENSv2 so leaving it unfixed cannot be construed as the DAO maliciously forcing users to v2.
Thank you, I’d somehow forgotten that this was previously disclosed.
I’d be very happy to see it fixed via the method I outlined. I don’t have time to write the (albeit simple) contract for this right now, but if anyone else is prepared to take it on I’m happy to review and provide feedback.
My understanding is that there is an active development team for ENS. What is the process to get that team (whoever they may be) to fix critical (IMO) bugs like these?
It appears that the previous report of these two bugs got dropped on the floor for some reason (the conversation just ends suddenly), and I would like to try to make sure that doesn’t happen again, but I’m not sure what the right process is to get development resources tasked to this issue.
Right now we are in the final weeks of preparing for the first dedicated ENS conference and DevCon, and I personally judged the risk of the DAO turning evil in the next couple of weeks as quite low. I will be happy to take this on once we are not trying to prepare for the largest event in the Ethereum calendar.
I agree that the risk of the DAO turning malicious within the next 2 weeks is quite low. A commitment to get this fixed “in the near future” (after conference) is a lot better than “in the undefined distant future when ENSv2 is launched”, and I am willing to accept that and wait for a few weeks before continuing to push on this.
Separately: I believe the much the bigger risk is governance takeover (someone buying enough ENS to attack), governance compromise (hacking one or more top delegates), governance attack (submitting an underhanded proposal masked with a positive proposal), or coercion (states coercing one or more delegates). Governance turning evil is a long slow process; maybe shorter than the time it takes to develop ENSv2, but longer than the time it takes to run a conference.
ENS now has a security council implemented for the next 2 years, so it’s safe to say that during these next two years, a proposal that would go against the ENS DAO constitution (which would need to happen for this NameWrapper vulnerability to be exploited) will not pass.
By then, ENSv2 will probably be much more defined, if not live.
By “underhanded proposal masked with a positive proposal” I meant that some proposal that has a legitimate looking code change that everyone thinks is fine, but contains a subtle bug (e.g., overflow or something) that allows the attacker to take some malicious action beyond the proposal’s intent. Keep in mind, the two bugs we are discussing here were not noticed in any of the ENS pre-launch audits, or by the ENS contract developers so it is very possible to have a bug that introduces a vulnerability which no one notices.