Unpriveleged user cannot renew fuse expiry

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/wrapper/NameWrapper.sol#L271-L275

Vulnerability details

Impact

The renew() function in NameWrapper.sol is only authorised to be called by a controller. This means that it is impossible for a user to renew their fuse expiry as EthRegistrarController.sol does not make a call to NameWrapper.renew(). Therefore when a user's subdomain fuse expires, it allows for the parent to transfer away the subdomain without the user's permission thereby breaking the trustless nature of the protocol

Proof of Concept

Scenario #1:

1. Alice owns test.eth and creates the subdomain www.test.eth for bob with the fuse PARENT_CANNOT_CONTROL burned
2. Bob uses his subdomain
3. When bob's subdomain expires, bob is not able to renew his subdomain causing the PARENT_CANNOT_CONTROL fuse to dissapear
4. Alice can now claim ownership away from bob's subdomain and use it for malicious purposes (identity theft, etc..)
   Scenario Gas Optimizations #2:
5. Alice owns test.eth and sets a certain fuse expiry date
6. After a certain time, the expiry date is reached and all fuses are burned
7. Alice has no method of renewing her fuses thereby losing functionality

Tools Used

VS Code

Recommended Mitigation Steps

In the renew() method call in EthRegistrarController.sol, add a call to renew() in NameWrapper.sol. Then make sure to remove one of the calls to registrar.renew() so that you don't increase the duration of the domain name by more than expected

[csanuragjain]

#63 and #202 & #54 seems to be duplicate of this issue

[jefflau]

This is a duplicate, but the proof of concept is related to subdomains, not .eth domains. renew() won’t be called by Bob in this example, because he owns www.test.eth, which is a subdomain of an .eth name and so is not related to the .eth registrar. Recommend reducing severity to medium

[Arachnid]

In fact, this should only be severity QA, as it can be worked around by calling renew on the registrar controller followed by setChildFuses.

[dmvt]

This report is invalid as it does not deal with a second, but a third level domain in the example. Third level domains (subdomains) are not renewed in this way.

[scaraven]

In my POC, I have outlined two scenarios, although it looks a bit strange due to the way GitHub deals with hashtags, the second does deal with second level domains.

[Arachnid]

Alice can renew her domain by calling renew; there is special-case code to handle allowing 2LD owners to renew their own domains.