ENS contest - fatherOfBlocks's results

Lines of code

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/ETHRegistrarController.sol#L183 https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/ETHRegistrarController.sol#L204

Vulnerability details

Impact

address.transfer by default can only send up to 2300 gas, to not allow reentrants. Now, if msg.value - (price.base + price.premium) > 2300, then the user would lose that gas.

Recommended Mitigation Steps

It is recommended to directly use the low-level transfer, call, and validate that it returns true on delivery.

dnssec-oracle/BytesUtils.sol

• NATSPEC IS INCOMPLETE

dnssec-oracle/DNSSECImpl.sol

• NATSPEC IS INCOMPLETE

dnssec-oracle/Owned.sol

• L9 - gas could be saved if instead of using a modifier a private view function is used.

• Consider adding a two-phase transfer, where the current owner nominates the next owner, and the next owner has to call accept*() to become the new owner. This prevents passing the ownership to an account that is unable to use it.

• FILE IS MISSING NATSPEC

wrapper/BytesUtil.sol

• The file is called BytesUtil.sol but the library is called BytesUtils. The correct thing would be that the file has the same name

wrapper/ERC1155Fuse.sol

• NATSPEC IS INCOMPLETE

wrapper/NameWrapper.sol

• NATSPEC IS INCOMPLETE

wrapper/Controllable.sol

• FILE IS MISSING NATSPEC

registry/ENS.sol

• The file is called ENS, but internally it has an interface, therefore it would be more consistent if the name is IENS. That it does not have the I of interface, generates confusion.

resolvers/Resolver.sol

• The file is called Resolver, but internally it has an interface, therefore it would be more consistent if the name is IResolver. That it does not have the I of interface, generates confusion.

ethregistrar/ETHRegistrarController.sol

• L49 - Several immutable variables are set in the constructor, if any input is zero, it should be deployed again since DoS would occur in different functions, therefore, it is recommended to validate that those addresses are != 0.

• L121 - It is validated with a require if it meets a condition, but a message according to the reason why the tx cannot be executed is not displayed.

• L270 - The function requests its inputs in one order, but inside it is used in another order, it is recommended that it be used in the same organic order as it is used.

dnssec-oracle/BytesUtils.sol

• L116/126 - An array.length variable could be generated in memory and used directly, this would generate a lower gas cost.

• L56/264/266 - It is not necessary to create a variable and set its default value of its type, you can save gas without setting it directly.

• L266/271/292/313 - Instead of i++ or i + 1 or i--, it is less expensive to do --i or ++i

dnssec-oracle/DNSSECImpl.sol

• L93/186/187 - You could generate a variable in memory of array.length and use that variable directly, this would generate a lower gas cost.

• L93 - It is not necessary to create a variable and set its default value of its type, you can save gas without setting it directly.

• L93 - Instead of i++ or i + 1 or i--, it is less expensive to do --i or ++i

dnssec-oracle/RRUtils.sol

• L39/40 - It is not necessary to create a variable if it will only be used once within the function.

• L50/181/200/310 - It is not necessary to create a variable and set its default value of its type, you can save gas without setting it directly.

• L24/54/235/241/250/260/271 - Instead of i++ or i + 1 or i--, it is less expensive to do --i or ++i

• L245 - Less gas is spent making variable != 0, than making variable > 0

• L94/307/310/315/316 - A variable could be generated in memory of array.length and use that variable directly, this would generate a lower gas cost.

ethregistrar/StringUtils.sol

• L12/14 - It is not necessary to create a variable and set its default value of its type, you can save gas without setting it directly.

• L14 - Instead of variable++, gas is saved by making ++variable.

registry/ReverseRegistrar.sol

• L41/52 - Instead of using a require, you can save gas using ifs with custom errors.

• L46/54 - When a string exceeds 32 bytes they generate a higher gas cost, try to generate messages with less than 32 characters.

wrapper/BytesUtil.sol

• L44 - Less gas is spent making variable != 0, than making variable > 0

• L28/45/49 - Instead of doing variable + 1 or variable - 1, you save gas by doing ++variable or --variable.

wrapper/ERC1155Fuse.sol

• L62/87/109/176/179/197/199/202/217/252/292/322/327/354/359 - When a string exceeds 32 bytes, they generate a higher gas cost, try to generate messages with less than 32 characters.

• L60/85/107/176/177/195/199/200/248/249/250/290 - gas could be saved if instead of using a modifier a private view function is used.

• L86/90/92/196/205 - array.length is used several times in the balanceOfBarch() and safeBatchTransferFrom() functions and between them in a for loop, gas could be saved if a length variable was created in memory, instead of querying the length each time.

• L92/205 - It is not necessary to create a variable and set its default value of its type, you can save gas without setting it directly.

• L92/205 - Gas can be saved by making the ++i unchecked.

wrapper/NameWrapper.sol

• L150/639/657 - gas could be saved if instead of using a modifier a private view function is used.

• L259/260/406/407/408/435/436/437/659/661 - It is not necessary to create a variable if it will only be used once within the function.

• L746/749/752 - You could generate a bytes(label).length variable in memory and use that variable directly, this would generate a lower gas cost.

wrapper/Controllable.sol

• L17 - When a string exceeds 32 bytes they generate a higher gas cost, try to generate messages with less than 32 characters.

• L16 - gas could be saved if instead of using a modifier a private view function is used.

• L17 - Instead of using a require, you can save gas using ifs with custom errors.

ethregistrar/ETHRegistrarController.sol

• L73/74/82/83/97/107 - It is not necessary to create a variable if it will only be used once within the function.

• L98 - Less gas is spent making variable != 0, than making variable > 0

• L101/139/198/234/240/242/261/265 - When a string exceeds 32 bytes they generate a higher gas cost, try to generate messages with less than 32 characters.

• L57/99/121/137/196/232/238/242/246/259 - Instead of using a require, you can save gas using ifs with custom errors.

• L256 - array.length is used in a for loop, gas could be saved if a length variable was created in memory, instead of length being consulted each time.

• L256 - It is not necessary to create a variable and set its default value of its type, you can save gas without setting it directly.

• L256 - Gas can be saved by making the ++i unchecked.