ENS contest - Funen's results

1. Comment was not the same as actual code

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/BytesUtils.sol#L178-L183

the actual was used the 20 Bytes

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/BytesUtils.sol#L184-L188

which mean it can be changed for comment instead to same as actual code.

2. Innacurate reason string

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/wrapper/ERC1155Fuse.sol#L249

the reason should be "ERC1155: can't mint zero address".

3. Missing Indexed

bytes name https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/wrapper/INameWrapper.sol#L21

4. owner_only can be changed

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/Owned.sol#L18

it supposed can be changed into onlyOwner then owner_only

5. using pragma abicoder V2

Since DNSSECImpl.sol and DNSSEC.solwas using pragma experimental ABIEncoderV2 and used pragma ^0.7.6, it because the ABI coder v2 is not considered experimental anymore, it can be selected via pragma abicoder v2 instead since Solidity 0.7.4.

POC

https://docs.soliditylang.org/en/latest/080-breaking-changes.html https://docs.soliditylang.org/en/latest/layout-of-source-files.html?highlight=experimental#abiencoderv2

Tools

Manual Review

Recommended Mitigation

Add pragma abicoder v1; if you want to stay with the old ABI coder. Optionally remove pragma experimental ABIEncoderV2 or pragma abicoder v2 since it is redundant.

6. Avoid Floatin Pragma's

Since it was used ^0.8.4, >=0.8.4, ^0.8.13. As the compiler can be use for example 0.8.x and consider locking at this version the same as another. It can be consider using locking the pragma version whenever possible and avoid using a floating pragma in the final deployment. Since it can be problematic, if there are publicly disclosed bugs and issues that affect the current compiler version used.

7. Require State need Reason String

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/Owned.sol#L10

        require(msg.sender == owner);

8. Use SPDX license identifier

For some contracts that was not used SPDX license identifier in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code

Occurances :

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/registry/ENS.sol https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/IMetadataService.sol https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/Owned.sol https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/algorithms/Algorithm.sol

9. Typo Comment

1.) Iff to if

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/DNSSECImpl.sol#L231

2.) Iff to if

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/DNSSECImpl.sol#L333

3.) Iff to if

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/algorithms/Algorithm.sol#L12

10. Value constant CAN_DO_EVERYTHING can be set on up

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/wrapper/INameWrapper.sol#L16

Since to be the same as another contracts and the value can be increasing so it should be CAN_DO_EVERYTHING in the upper of CANNOT_UNWRAP

1. Short reason string can be used for saving more gas

Every reason string takes at least 32 bytes. Use short reason strings that fits in 32 bytes or it will become more expensive.

main/contracts/dnssec-oracle/RRUtils.sol#L307  "Long keys not permitted"
main/contracts/wrapper/Controllable.sol#L17 "Controllable: Caller is not a controller"
main/contracts/wrapper/BytesUtil.sol#L28  "namehash: Junk at end of name"
main/contracts/wrapper/BytesUtil.sol#L42 "readLabel: Index out of bounds"
main/contracts/wrapper/ERC1155Fuse.sol#L62 "ERC1155: balance query for the zero address"
main/contracts/wrapper/ERC1155Fuse.sol#L87 "ERC1155: accounts and ids length mismatch"
main/contracts/wrapper/ERC1155Fuse.sol#L109 "ERC1155: setting approval status for self"
main/contracts/wrapper/ERC1155Fuse.sol#L176 "ERC1155: transfer to the zero address"
main/contracts/wrapper/ERC1155Fuse.sol#L179 "ERC1155: caller is not owner nor approved"
main/contracts/wrapper/ERC1155Fuse.sol#L197"ERC1155: ids and amounts length mismatch"
main/contracts/wrapper/ERC1155Fuse.sol#L199 "ERC1155: transfer to the zero address"
main/contracts/wrapper/ERC1155Fuse.sol#L202 "ERC1155: transfer caller is not owner nor approved"
main/contracts/wrapper/ERC1155Fuse.sol#L217 "ERC1155: insufficient balance for transfer"
main/contracts/wrapper/ERC1155Fuse.sol#L322 "ERC1155: ERC1155Receiver rejected tokens"
main/contracts/wrapper/ERC1155Fuse.sol#L327 "ERC1155: transfer to non ERC1155Receiver implementer"
main/contracts/wrapper/ERC1155Fuse.sol#L354 "ERC1155: ERC1155Receiver rejected tokens"
main/contracts/wrapper/ERC1155Fuse.sol#L359 "ERC1155: transfer to non ERC1155Receiver implementer"

2. Using count++ can saving more gas

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/RRUtils.sol#L58

3. Saving gas by removing = 0

This implementation code can be saving more gas by removing = 0, it because If a variable was not set/initialized, it is assumed to have default value to 0

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/wrapper/ERC1155Fuse.sol#L145 https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/BytesUtils.sol#L264

4. change uint256 i = 0 into uint i for saving more gas

using this implementation can saving more gas for each loops.

/main/contracts/wrapper/ERC1155Fuse.sol#L92         for (uint256 i = 0; i < accounts.length; ++i) {
/main/contracts/wrapper/ERC1155Fuse.sol#L205         for (uint256 i = 0; i < ids.length; ++i) {
 /main/contracts/dnssec-oracle/BytesUtils.sol#L266        for(uint i = 0; i < len; i++) {