File: contracts\wrapper\NameWrapper.sol
229:
230: // transfer the token from the user to this contract
231: registrar.transferFrom(registrant, address(this), tokenId);
232:
233: // transfer the ens record back to the new owner (this contract)
234: registrar.reclaim(tokenId, address(this));
235:
2. call() should be used instead of transfer() on an address payable
Sometimes this kind of issue is considered as Medium risk.
The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:
The claimer smart contract does not implement a payable function.
The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.
Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.
File: contracts\wrapper\NameWrapper.sol
367: /**
368: * @notice Sets fuses of a name
369: * @param node namehash of the name
370: * @param fuses fuses to burn (cannot burn PARENT_CANOT_CONTROL)
371: */
372:
373: function setFuses(bytes32 node, uint32 fuses)
PARENT_CANOT_CONTROL should be PARENT_CANNOT_CONTROL
5. Wrong comparison result when the length is longer than 32
The comparison will be wrong when then shortest > 32 because the mask is wrong.
For example when the parameters are 01234567890123456789012345678901xaxa, 0, 3501234567890123456789012345678901xaxb, 0, 35, the result should be zero because they are same with the first 35 characters. For the 2nd iteration of L56, the shortest is greater than 32, and the mask will be type(uint256).max and it will mask all values and this will result to diff != 0.
Recommended Mitigration: compare shortest-idx to 32 at line L66
When self.length > offset + other.length, the result can be true.
For example when the parameters are hello1, 1, ello, the result should be false because ello1 is different from ello.
But the result will be true with this function because the equals function will compare the string within the len.
1. Use safeTransferFrom instead of transferFrom for ERC721 transfers
OpenZeppelin’s documentation discourages the use of
transferFrom()
, usesafeTransferFrom()
whenever possible. https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L231https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L341
2. call() should be used instead of transfer() on an address payable
Sometimes this kind of issue is considered as Medium risk.
The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:
Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L182
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L203
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L210
3. Unbounded loops with external calls
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L167
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/BulkRenewal.sol#L56
4. Wrong Comment
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L370
PARENT_CANOT_CONTROL
should bePARENT_CANNOT_CONTROL
5. Wrong comparison result when the length is longer than 32
The comparison will be wrong when then
shortest
> 32 because themask
is wrong. For example when the parameters are01234567890123456789012345678901xaxa
,0
,35
01234567890123456789012345678901xaxb
,0
,35
, the result should be zero because they are same with the first 35 characters. For the 2nd iteration ofL56
, the shortest is greater than 32, and the mask will betype(uint256).max
and it will mask all values and this will result todiff != 0
.shortest-idx
to32
at lineL66
5. Wrong comparison result when the
self
is longer than otherhttps://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L116
When
self.length > offset + other.length
, the result can be true. For example when the parameters arehello1
,1
,ello
, the result should befalse
becauseello1
is different fromello
. But the result will betrue
with this function because the equals function will compare the string within thelen
.