Reentrancy for function call before state update

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L819-L821

Vulnerability details

Impact

An external call "_transfer" is made before updating state data through "_setFuses" and "_setFuses" does not depend on any data from "_transfer".

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L819-L821.

Proof of Concept

Reentrancy is not only an effect of Ether transfer but of any function call on another contract.

The contract should make use of the Checks-Effects-Interactions Pattern.

Recommended Mitigation Steps

Update the state before making the external call.

$ git diff --no-index NameWrapper.sol.orig NameWrapper.sol
diff --git a/NameWrapper.sol.orig b/NameWrapper.sol
index b652447..d0dfa6f 100644
--- a/NameWrapper.sol.orig
+++ b/NameWrapper.sol
@@ -817,8 +817,8 @@ contract NameWrapper is
         uint64 expiry
     ) internal {
         (address owner, , ) = getData(uint256(node));
-        _transfer(owner, newOwner, uint256(node), 1, "");
         _setFuses(node, newOwner, fuses, expiry);
+        _transfer(owner, newOwner, uint256(node), 1, "");
     }

[sseefried]

Duplicate of #124

[dmvt]

Duplicate of #84