Reentrancy from _transferAndBurnFuses

[code423n4]

Lines of code

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

Vulnerability details

Impact

Reentrancy attack

Proof of Concept

The function _transferAndBurnFuses is not performing Checks-Effects-Interactions pattern, and updates fuses after NFTs are transfered.
An attacker can reenter because _transfer is performing a _doSafeTransferAcceptanceCheck where it check the receive is able to handle NFTs.

function _transferAndBurnFuses(
        bytes32 node,
        address newOwner,
        uint32 fuses,
        uint64 expiry
    ) internal {
        (address owner, , ) = getData(uint256(node));
        _transfer(owner, newOwner, uint256(node), 1, "");  @audit here is the transfer
        _setFuses(node, newOwner, fuses, expiry);@audut here we set a storage variable
    }

Recommended Mitigation Steps

Set the variables before transferring

​ [-] transfer(owner, newOwner, uint256(node), 1, "");
​ [-] _setFuses(node, newOwner, fuses, expiry);

    [+]_ setFuses(node, newOwner, fuses, expiry);

​ [+] _transfer(owner, newOwner, uint256(node), 1, "");
​

[sseefried]

Duplicate of #124

[jefflau]

Duplicate of #117

[dmvt]

Duplicate of #84