Renew of 2nd level domain is not done properly

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L201 https://github.com/ensdomains/ens-contracts/blob/master/contracts/wrapper/NameWrapper.sol#L271

Vulnerability details

Impact

The ETHRegistrarController is calling renew from base registrar and not through Namewrapper. This means the fuses for the subdomain will not be updated via _setData. This impacts the permission model set over subdomain and could lead to takeover

---

Proof of Concept

1. Observe the renew function

function renew(string calldata name, uint256 duration)
        external
        payable
        override
    {
        ...

        uint256 expires = base.renew(uint256(label), duration);

        ....
    }

2. As we can see this is calling renew function of Base Registrar instead of NameWrapper. Since this is not going via NameWrapper fuses will not be set

3. Also since renew in NameWrapper can only be called via Controller which is ETHRegistrarController so there is no way to renew subdomain

---

Recommended Mitigation Steps

The ETHRegistrarController must renew using Namewrapper's renew contract

[Arachnid]

Duplicate of #223.

[dmvt]

On #223, which I've invalidated, @Arachnid notes that:

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

I'm going to make this report the main one and leave the risk rating of Medium in place. While there is a workaround, if the workaround is not employed, permissions will be incorrect and may lead to a breakdown in the functioning of the protocol.