Skip to content

Instantly share code, notes, and snippets.

@liveactionllama
Created December 15, 2023 00:42
Show Gist options
  • Save liveactionllama/6a852871036a324c234151ba02fa2377 to your computer and use it in GitHub Desktop.
Save liveactionllama/6a852871036a324c234151ba02fa2377 to your computer and use it in GitHub Desktop.
Gist to replace broken link

Gas Summary

Gas Optimizations

Issue Contexts Estimated Gas Saved
GAS‑1 abi.encode() is less efficient than abi.encodepacked() 1 100
GAS‑2 <array>.length Should Not Be Looked Up In Every Loop Of A For-loop 2 194
GAS‑3 Use calldata instead of memory for function parameters 6 1800
GAS‑4 Setting the constructor to payable 3 39
GAS‑5 Duplicated require()/revert() Checks Should Be Refactored To A Modifier Or Function 2 56
GAS‑6 Using delete statement can save gas 10 -
GAS‑7 ++i Costs Less Gas Than i++, Especially When It’s Used In For-loops (--i/i-- Too) 5 30
GAS‑8 Functions guaranteed to revert when called by normal users can be marked payable 3 63
GAS‑9 Use hardcoded address instead address(this) 4 -
GAS‑10 It Costs More Gas To Initialize Variables To Zero Than To Let The Default Of Zero Be Applied 4 -
GAS‑11 internal functions only called once can be inlined to save gas 63 1386
GAS‑12 Optimize names to save gas 9 198
GAS‑13 <x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables 25 -
GAS‑14 Public Functions To External 6 -
GAS‑15 require() Should Be Used Instead Of assert() 3 -
GAS‑16 require()/revert() Strings Longer Than 32 Bytes Cost Extra Gas 3 -
GAS‑17 Save gas with the use of specific import statements 44 -
GAS‑18 Shorten the array rather than copying to a new one 1 -
GAS‑19 Splitting require() Statements That Use && Saves Gas 1 9
GAS‑20 Help The Optimizer By Saving A Storage Variable’s Reference Instead Of Repeatedly Fetching It 1 -
GAS‑21 Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead 40 -
GAS‑22 ++i/i++ Should Be unchecked{++i}/unchecked{i++} When It Is Not Possible For Them To Overflow, As Is The Case When Used In For- And While-loops 7 245
GAS‑23 Using unchecked blocks to save gas 4 80
GAS‑24 Use v4.8.1 OpenZeppelin contracts 1 -
GAS‑25 Use solidity version 0.8.19 to gain some gas boost 19 1672
GAS‑26 Use uint256(1)/uint256(2) instead for true and false boolean states 2 10000

Total: 269 contexts over 26 issues

Gas Optimizations

See for more information: https://github.com/ConnorBlockchain/Solidity-Encode-Gas-Comparison

Proof Of Concept

61: abi.encode(name, data)

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L61

The overheads outlined below are PER LOOP, excluding the first loop

storage arrays incur a Gwarmaccess (100 gas) memory arrays use MLOAD (3 gas) calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP (3 gas), and gets rid of the extra DUP needed to store the stack offset

Proof Of Concept

118: for (uint256 i = 0; i < input.length; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L118

384: for (uint256 i = 0; i < data.length + 31; i += 32) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L384

In some cases, having function arguments in calldata instead of memory is more optimal.

Consider the following generic example:

contract C {
	function add(uint[] memory arr) external returns (uint sum) {
		uint length = arr.length;
		for (uint i = 0; i < arr.length; i++) {
		    sum += arr[i];
		}
	}
}

In the above example, the dynamic array arr has the storage location memory. When the function gets called externally, the array values are kept in calldata and copied to memory during ABI decoding (using the opcode calldataload and mstore). And during the for loop, arr[i] accesses the value in memory using a mload. However, for the above example this is inefficient. Consider the following snippet instead:

contract C {
	function add(uint[] calldata arr) external returns (uint sum) {
		uint length = arr.length;
		for (uint i = 0; i < arr.length; i++) {
		    sum += arr[i];
		}
	}
}

In the above snippet, instead of going via memory, the value is directly read from calldata using calldataload. That is, there are no intermediate memory operations that carries this value.

Gas savings: In the former example, the ABI decoding begins with copying value from calldata to memory in a for loop. Each iteration would cost at least 60 gas. In the latter example, this can be completely avoided. This will also reduce the number of instructions and therefore reduces the deploy time cost of the contract.

In short, use calldata instead of memory if the function argument is only read.

Note that in older Solidity versions, changing some function arguments from memory to calldata may cause "unimplemented feature error". This can be avoided by using a newer (0.8.*) Solidity compiler.

Examples Note: The following pattern is prevalent in the codebase:

function f(bytes memory data) external {
	(...) = abi.decode(data, (..., types, ...));
}

Here, changing to bytes calldata will decrease the gas. The total savings for this change across all such uses would be quite significant.

Proof Of Concept

function proveAndClaim(
        bytes memory name,
        DNSSEC.RRSetWithSignature[] memory input
    ) public override {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L90

function proveAndClaimWithResolver(
        bytes memory name,
        DNSSEC.RRSetWithSignature[] memory input,
        address resolver,
        address addr
    ) public override {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L101

function _claim(
        bytes memory name,
        DNSSEC.RRSetWithSignature[] memory input
    ) internal returns (bytes32 parentNode, bytes32 labelHash, address addr) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L133

function resolve(
        bytes memory name,
        uint16 qtype
    ) external returns (DNSSEC.RRSetWithSignature[] memory);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L23

function verifyRRSet(
        RRSetWithSignature[] memory input
    )
        external
        view
        virtual
        override
        returns (bytes memory rrs, uint32 inception)
    {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L87

function verifyRRSet(
        RRSetWithSignature[] memory input,
        uint256 now
    )
        public
        view
        virtual
        override
        returns (bytes memory rrs, uint32 inception)
    {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L87

Saves ~13 gas per instance

Proof Of Concept

55: constructor(
        address _previousRegistrar,
        address _resolver,
        DNSSEC _dnssec,
        PublicSuffixList _suffixes,
        ENS _ens
    )

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L55

43: constructor(ENS _ens, DNSSEC _oracle, string memory _gatewayURL)

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L43

52: constructor(bytes memory _anchors)

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L52

Saves deployment costs

Proof Of Concept

18: require(offset + len <= self.length);
305: require(offset + len <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L18

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L305

Proof Of Concept

339: uint256 ret = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L339

59: uint256 count = 0;
72: uint256 constant RRSIG_TYPE = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L59

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L72

210: uint256 constant DNSKEY_FLAGS = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L210

236: uint256 constant DS_KEY_TAG = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L236

263: uint256 off = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L263

12: uint8 labelLength = 0;
16: node = 0;
18: dnsName[0] = 0;
12: labelLength = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L12

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L16

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L18

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L12

Saves 6 gas per loop

Proof Of Concept

341: for (uint256 i = 0; i < len; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L341

393: for (uint256 idx = off; idx < off + len; idx++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L393

118: for (uint256 i = 0; i < input.length; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L118

325: for (uint256 i = 0; i < exp; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L325

25: for (uint256 i = length - 1; i >= 0; i--) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L25

Recommended Mitigation Steps

For example, use ++i instead of i++

If a function modifier or require such as onlyOwner/onlyX is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

Proof Of Concept

80: function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

64: function setAlgorithm(uint8 id, Algorithm algo) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L64

75: function setDigest(uint8 id, Digest digest) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L75

Recommended Mitigation Steps

Functions guaranteed to revert when called by normal users can be marked payable.

Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundry's script.sol and solmate's LibRlp.sol contracts can help achieve this.

References: https://book.getfoundry.sh/reference/forge-std/compute-create-address

https://twitter.com/transmissions11/status/1518507047943245824

Proof Of Concept

190: root.setSubnodeOwner(label, address(this));
196: address(this),
201: } else if (owner != address(this)) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L190

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L196

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L201

57: address(this),

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L57

Recommended Mitigation Steps

Use hardcoded address

Proof Of Concept

341: for (uint256 i = 0; i < len; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L341

118: for (uint256 i = 0; i < input.length; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L118

384: for (uint256 i = 0; i < data.length + 31; i += 32) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L384

325: for (uint256 i = 0; i < exp; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L325

Proof Of Concept

19: function getOwnerAddress

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L19

46: function parseRR

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L46

66: function parseString

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L66

136: function parseRR

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L136

162: function readTXT

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L162

190: function resolveName

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L190

179: function readUint8

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L179

192: function readUint16

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L192

208: function readUint32

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L208

224: function readBytes32

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L224

240: function readBytes20

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L240

260: function readBytesN

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L260

300: function substring

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L300

332: function base32HexDecodeWord

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L332

387: function find

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L387

140: function validateSignedSet

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L140

181: function validateRRs

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L181

225: function verifySignature
285: function verifySignature

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L225

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L285

254: function verifyWithKnownKey

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L254

330: function verifyWithDS

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L330

373: function verifyKeyWithDS

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L373

415: function verifyDSHash

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L415

41: function readName

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L41

94: function readSignedSet

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L94

111: function rrs

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L111

136: function iterateRRs

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L136

150: function done

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L150

158: function next

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L158

19: function name
187: function name

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L19

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L187

200: function rdata

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L200

222: function readDNSKEY

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L222

248: function readDS

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L248

259: function isSubdomainOf

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L259

275: function compareNames

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L275

332: function serialNumberGte

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L332

353: function computeKeytag
358: function computeKeytag

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L353

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L358

6: function sha1

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/SHA1.sol#L6

65: function toProjectivePoint

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L65

77: function addAndReturnProjectivePoint

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L77

117: function zeroAffine

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L117

137: function isOnCurve

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L137

77: function add
208: function add
247: function add
286: function add

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L77

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L208

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L247

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L286

159: function twice
302: function twice

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L159

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L302

316: function multiplyPowerBase2

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L316

377: function multipleGeneratorByScalar

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L377

386: function validateSignature

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L386

7: function modexp

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/ModexpPrecompile.sol#L7

30: function parseSignature

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L30

37: function parseKey

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L37

14: function rsarecover

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSAVerify.sol#L14

11: function hexStringToBytes32

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L11

68: function hexToAddress

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L68

9: function dnsEncodeName

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L9

12: function keccak

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L12

29: function namehash

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L29

49: function readLabel

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L49

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.

See more here

Proof Of Concept

All in-scope contracts

Recommended Mitigation Steps

Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Gauge.sol contract will be the most used; A lower method ID may be given.

Proof Of Concept

53: idx += 1;
60: idx += len;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L53

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L60

77: for (uint256 idx = 0; idx < shortest; idx += 32) {
95: selfptr += 32;
96: otherptr += 32;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L77

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L95

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L96

275: for (; len >= 32; len -= 32) {
279: dest += 32;
280: src += 32;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L275

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L279

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L280

359: bitlen -= 2;
363: bitlen -= 4;
367: bitlen -= 1;
371: bitlen -= 3;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L359

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L363

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L367

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L371

27: idx += labelLen + 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L27

63: offset += labelLen + 1;
67: count += 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L63

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L67

169: off += 2;
169: off += 2;
173: off += 4;
169: off += 2;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L169

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L169

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L173

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L169

309: counts -= 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L309

384: for (uint256 i = 0; i < data.length + 31; i += 32) {
393: ac1 +=
397: ac2 += (word &
429: ac1 += (ac1 >> 16) & 0xFFFF;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L384

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L393

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L397

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L429

36: labelLength += 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L36

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

Proof Of Concept

function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

function proveAndClaim(
        bytes memory name,
        DNSSEC.RRSetWithSignature[] memory input
    ) public override {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L90

function proveAndClaimWithResolver(
        bytes memory name,
        DNSSEC.RRSetWithSignature[] memory input,
        address resolver,
        address addr
    ) public override {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L101

function enableNode(bytes memory domain) public returns (bytes32 node) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L166

function setAlgorithm(uint8 id, Algorithm algo) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L64

function setDigest(uint8 id, Digest digest) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L75

Proof Of Concept

169: assert(startIdx + fieldLength < lastIdx);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L169

25: assert(idx < self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L25

61: assert(offset < self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L61

Proof Of Concept

33: require(data.length == 64, "Invalid p256 signature length");

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L33

35: require(offset == self.length - 1, "namehash: Junk at end of name");

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L35

53: require(idx < self.length, "readLabel: Index out of bounds");

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L53

The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files

A good example:

import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {SafeCastLib} from "solmate/utils/SafeCastLib.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IProducer} from "src/interfaces/IProducer.sol";
import {GlobalState, UserState} from "src/Common.sol";

Proof Of Concept

4: import "../dnssec-oracle/DNSSEC.sol";
5: import "../dnssec-oracle/BytesUtils.sol";
6: import "../dnssec-oracle/RRUtils.sol";
7: import "../utils/HexUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L4-L7

7: import "../dnssec-oracle/BytesUtils.sol";
8: import "../dnssec-oracle/DNSSEC.sol";
9: import "../dnssec-oracle/RRUtils.sol";
10: import "../registry/ENSRegistry.sol";
11: import "../root/Root.sol";
12: import "../resolvers/profiles/AddrResolver.sol";
13: import "./DNSClaimChecker.sol";
14: import "./PublicSuffixList.sol";
15: import "./IDNSRegistrar.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L7-L15

4: import "../../contracts/resolvers/profiles/IAddrResolver.sol";
5: import "../../contracts/resolvers/profiles/IExtendedResolver.sol";
6: import "../../contracts/resolvers/profiles/IExtendedDNSResolver.sol";
8: import "../dnssec-oracle/BytesUtils.sol";
9: import "../dnssec-oracle/DNSSEC.sol";
10: import "../dnssec-oracle/RRUtils.sol";
11: import "../registry/ENSRegistry.sol";
12: import "../utils/HexUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L4

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L5

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L6

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L8

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L9

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L10

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L11

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L12

4: import "../dnssec-oracle/BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/RecordParser.sol#L4

5: import "./Owned.sol";
6: import "./BytesUtils.sol";
7: import "./RRUtils.sol";
8: import "./DNSSEC.sol";
9: import "./algorithms/Algorithm.sol";
10: import "./digests/Digest.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L5-L10

3: import "./BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L3

3: import "./Algorithm.sol";
4: import "./EllipticCurve.sol";
5: import "../BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L3-L5

3: import "./Algorithm.sol";
4: import "../BytesUtils.sol";
5: import "./RSAVerify.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L3-L5

3: import "./Algorithm.sol";
4: import "../BytesUtils.sol";
5: import "./RSAVerify.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA256Algorithm.sol#L3-L5

3: import "../BytesUtils.sol";
4: import "./ModexpPrecompile.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSAVerify.sol#L3-L4

3: import "./Digest.sol";
4: import "../BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA1Digest.sol#L3-L4

3: import "./Digest.sol";
4: import "../BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA256Digest.sol#L3-L4

Recommended Mitigation Steps

Use specific imports syntax per solidity docs recommendation.

Inline-assembly can be used to shorten the array by changing the length slot, so that the entries don't have to be copied to a new, shorter array

Proof Of Concept

53: string[] memory urls = new string[](1);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L53

Instead of using operator && on a single require. Using a two require can save more gas.

i.e. for require(version == 1 && _bytecodeHash[1] == bytes1(0), "zf"); use:

	require(version == 1);
	require(_bytecodeHash[1] == bytes1(0));

Proof Of Concept

343: require(char >= 0x30 && char <= 0x7A);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L343

To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array. The effect can be quite significant. As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.

Proof Of Concept

394: if (self[idx] == needle) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L394

When using elements that are smaller than 32 bytes, your contract's gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

Proof Of Concept

16: uint16 constant CLASS_INET = 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L16

17: uint16 constant TYPE_TXT = 16;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L17

44: uint64 ttl;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L44

130: interfaceID == type(IDNSRegistrar).interfaceId;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L130

29: uint16 constant CLASS_INET = 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L29

340: uint8 decoded;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L340

27: uint16 constant DNSCLASS_IN = 1;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L27

29: uint16 constant DNSTYPE_DS = 43;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L29

30: uint16 constant DNSTYPE_DNSKEY = 48;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L30

304: uint16 computedkeytag = keyrdata.computeKeytag();

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L304

379: uint16 keytag = keyrdata.computeKeytag();

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L379

82: uint16 typeCovered;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L82

243: uint8 algorithm;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L243

84: uint8 labels;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L84

125: uint32 ttl;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L125

86: uint32 expiration;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L86

87: uint32 inception;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L87

242: uint16 keytag;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L242

123: uint16 dnstype;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L123

124: uint16 class;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L124

216: uint16 flags;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L216

217: uint8 protocol;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L217

244: uint8 digestType;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L244

22: uint16 exponentLen = uint16(key.readUint8(4));

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L22

21: uint16 exponentLen = uint16(key.readUint8(4));

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA256Algorithm.sol#L21

12: uint8 labelLength = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L12

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas PER LOOP

Proof Of Concept

77: for (uint256 idx = 0; idx < shortest; idx += 32) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L77

341: for (uint256 i = 0; i < len; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L341

393: for (uint256 idx = off; idx < off + len; idx++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L393

118: for (uint256 i = 0; i < input.length; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L118

260: for (; !proof.done(); proof.next()) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L260

380: for (; !dsrrs.done(); dsrrs.next()) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L380

325: for (uint256 i = 0; i < exp; i++) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L325

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block

Proof Of Concept

346: if (i == len - 1) {
376: return bytes32(ret << (256 - bitlen));

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L346

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L376

390: uint256 unused = 256 - (data.length - i) * 8;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L390

35: require(offset == self.length - 1, "namehash: Junk at end of name");

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L35

The upcoming v4.8.1 version of OpenZeppelin provides many small gas optimizations. https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v4.8.1

Proof Of Concept

    "@openzeppelin/contracts": "^4.1.0"

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/../package.json#L59

Recommended Mitigation Steps

Update OpenZeppelin Contracts Usage in package.json and require at least version of 4.8.1

Upgrade to the latest solidity version 0.8.19 to get additional gas savings. See latest release for reference: https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/

Proof Of Concept

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L2

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L3

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L2

pragma solidity ^0.8.11;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/RecordParser.sol#L2

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L2

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L1

pragma solidity >=0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/SHA1.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/ModexpPrecompile.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA256Algorithm.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSAVerify.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA1Digest.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA256Digest.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L2

pragma solidity ^0.8.13;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L2

pragma solidity ~0.8.17;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L2

If you don't use boolean for storage you will avoid Gwarmaccess 100 gas. In addition, state changes of boolean from true to false can cost up to ~20000 gas rather than uint256(2) to uint256(1) that would cost significantly less.

Proof Of Concept

16: valid = true;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L16

53: valid := false

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L53

QA Summary

Low Risk Issues

Issue Contexts
LOW‑1 Low Level Calls With Solidity Version 0.8.14 Can Result In Optimiser Bug 2
LOW‑2 Contracts are not using their OZ Upgradeable counterparts 2
LOW‑3 Pragma Experimental ABIEncoderV2 is Deprecated 1
LOW‑4 Remove unused code 2
LOW‑5 require() should be used instead of assert() 3
LOW‑6 Admin privilege - A single point of failure can allow a hacked or malicious owner use critical functions in the project 1
LOW‑7 Upgrade OpenZeppelin Contract Dependency 1

Total: 12 contexts over 7 issues

Non-critical Issues

Issue Contexts
NC‑1 Add a timelock to critical functions 3
NC‑2 Avoid Floating Pragmas: The Version Should Be Locked 17
NC‑3 Variable Names That Consist Of All Capital Letters Should Be Reserved For Const/immutable Variables 2
NC‑4 Constants in comparisons should appear on the left side 9
NC‑5 Critical Changes Should Use Two-step Procedure 3
NC‑6 Declare interfaces on separate files 1
NC‑7 Duplicated require()/revert() Checks Should Be Refactored To A Modifier Or Function 2
NC‑8 Function writing that does not comply with the Solidity Style Guide 19
NC‑9 Use delete to Clear Variables 3
NC‑10 Imports can be grouped together 44
NC‑11 NatSpec return parameters should be included in contracts 1
NC‑12 No need to initialize uints to zero 7
NC‑13 Initial value check is missing in Set Functions 3
NC‑14 Contracts should have full test coverage 1
NC‑15 Implementation contract may not be initialized 3
NC‑16 NatSpec comments should be increased in contracts 1
NC‑17 Non-usage of specific imports 44
NC‑18 Use a more recent version of Solidity 19
NC‑19 Omissions in Events 1
NC‑20 Open TODOs 3
NC‑21 Using >/>= without specifying an upper bound is unsafe 1
NC‑22 Public Functions Not Called By The Contract Should Be Declared External Instead 4
NC‑23 require() / revert() Statements Should Have Descriptive Reason Strings 13
NC‑24 Use bytes.concat() 8

Total: 212 contexts over 24 issues

Low Risk Issues

The project contracts in scope are using low level calls with solidity version before 0.8.14 which can result in optimizer bug. https://medium.com/certora/overly-optimistic-optimizer-certora-bug-disclosure-2101e3f7994d

Simliar findings in Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#5-low-level-calls-with-solidity-version-0814-can-result-in-optimiser-bug

Proof Of Concept

POC can be found in the above medium reference url.

Functions that execute low level calls in contracts with solidity version under 0.8.14

1:   pragma solidity ^0.8.4;

...

276: assembly {
          mstore(dest, mload(src))
      }

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L276

Recommended Mitigation Steps

Consider upgrading to at least solidity v0.8.15.

The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behaviour.

Proof of Concept

5: import "@openzeppelin/contracts/utils/introspection/IERC165.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L5

7: import "@openzeppelin/contracts/utils/introspection/ERC165.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L7

Recommended Mitigation Steps

Where applicable, use the contracts from @openzeppelin/contracts-upgradeable instead of @openzeppelin/contracts. See https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/tree/master/contracts for list of available upgradeable contracts

Proof Of Concept

pragma experimental ABIEncoderV2

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L3

Recommended Mitigation Steps

Use pragma abicoder v2 instead.

This code is not used in the main project contract files, remove it or add event-emit Code that is not in use, suggests that they should not be present and could potentially contain insecure functionalities.

Proof Of Concept

function multiplyPowerBase2

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L316

function multipleGeneratorByScalar

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L377

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

Proof Of Concept

169: assert(startIdx + fieldLength < lastIdx);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L169

25: assert(idx < self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L25

61: assert(offset < self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L61

The owner role has a single point of failure and onlyOwner can use critical a few functions.

owner role in the project: Owner is not behind a multisig and changes are not behind a timelock.

Even if protocol admins/developers are not malicious there is still a chance for Owner keys to be stolen. In such a case, the attacker can cause serious damage to the project due to important functions. In such a case, users who have invested in project will suffer high financial losses.

Impact

Hacked owner or malicious owner can immediately use critical functions in the project.

Proof Of Concept

80: function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

Recommended Mitigation Steps

Add a time lock to critical functions. Admin-only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them and consider if they would like to engage/exit based on how they perceive the changes as affecting the trustworthiness of the protocol or profitability of the implemented financial services.

Allow only multi-signature wallets to call the function to reduce the likelihood of an attack.

https://twitter.com/danielvf/status/1572963475101556738?s=20&t=V1kvzfJlsx-D2hfnG0OmuQ

An outdated OZ version is used (which has known vulnerabilities, see: https://github.com/OpenZeppelin/openzeppelin-contracts/security/advisories).

Proof Of Concept

Require dependencies to be at least version of 4.8.1 to include critical vulnerability patches.

    "@openzeppelin/contracts": "^4.1.0"

https://github.com/code-423n4/2023-04-ens/tree/main/package.json#L59

Recommended Mitigation Steps

Update OpenZeppelin Contracts Usage in package.json and require at least version of 4.8.1 via >=4.8.1

Non Critical Issues

It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user). Consider adding a timelock to the following functions:

Proof Of Concept

80: function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

64: function setAlgorithm(uint8 id, Algorithm algo) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L64

75: function setDigest(uint8 id, Digest digest) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L75

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

Proof Of Concept

Found usage of floating pragmas ^0.8.4 of Solidity in [DNSClaimChecker.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L2

Found usage of floating pragmas ^0.8.4 of Solidity in [DNSRegistrar.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L3

Found usage of floating pragmas ^0.8.4 of Solidity in [OffchainDNSResolver.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L2

Found usage of floating pragmas ^0.8.11 of Solidity in [RecordParser.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/RecordParser.sol#L2

Found usage of floating pragmas ^0.8.4 of Solidity in [BytesUtils.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [DNSSECImpl.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L2

Found usage of floating pragmas ^0.8.4 of Solidity in [RRUtils.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [EllipticCurve.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [ModexpPrecompile.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/ModexpPrecompile.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [P256SHA256Algorithm.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [RSASHA1Algorithm.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [RSASHA256Algorithm.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA256Algorithm.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [RSAVerify.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSAVerify.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [SHA1Digest.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA1Digest.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [SHA256Digest.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA256Digest.sol#L1

Found usage of floating pragmas ^0.8.4 of Solidity in [HexUtils.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L2

Found usage of floating pragmas ^0.8.13 of Solidity in [NameEncoder.sol]

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L2

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead.

Proof Of Concept

142: uint256 LHS = mulmod(y, y, p);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L142

143: uint256 RHS = mulmod(mulmod(x, x, p), x, p);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L143

Doing so will prevent typo bugs

Proof Of Concept

179: if (len == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L179

28: if (labelLen == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L28

64: if (labelLen == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L64

312: if (off == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L312

315: if (otheroff == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L315

42: if (u == 0 || u == m || m == 0) return 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L42

340: if (scalar == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L340

17: if (length == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L17

39: if (i == 0) {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L39

The critical procedures should be two step process.

See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure

Proof Of Concept

80: function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

64: function setAlgorithm(uint8 id, Algorithm algo) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L64

75: function setDigest(uint8 id, Digest digest) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L75

Recommended Mitigation Steps

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

Interfaces should be declared on a separate file and not on the same file where the contract resides in.

Proof Of Concept

22: interface IDNSGateway {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L22

Saves deployment costs

Proof Of Concept

18: require(offset + len <= self.length);
305: require(offset + len <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L18

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L305

Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.

https://docs.soliditylang.org/en/v0.8.17/style-guide.html

Functions should be grouped according to their visibility and ordered:

  • constructor
  • receive function (if exists)
  • fallback function (if exists)
  • external
  • public
  • internal
  • private
  • within a grouping, place the view and pure functions last

Proof Of Concept

Several in-scope contracts

See OffchainDNSResolver.sol for example

delete a assigns the initial value for the type to a. i.e. for integers it is equivalent to a = 0, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x.

The delete key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete statements.

Proof Of Concept

16: node = 0;
18: dnsName[0] = 0;
12: labelLength = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L16

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L18

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L12

Consider importing OZ first, then ENS related files, then all interfaces, then all utils.

Proof Of Concept

4: import "../dnssec-oracle/DNSSEC.sol";
5: import "../dnssec-oracle/BytesUtils.sol";
6: import "../dnssec-oracle/RRUtils.sol";
7: import "../utils/HexUtils.sol";
8: import "@ensdomains/buffer/contracts/Buffer.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L4-L8

5: import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
6: import "@ensdomains/buffer/contracts/Buffer.sol";
7: import "../dnssec-oracle/BytesUtils.sol";
8: import "../dnssec-oracle/DNSSEC.sol";
9: import "../dnssec-oracle/RRUtils.sol";
10: import "../registry/ENSRegistry.sol";
11: import "../root/Root.sol";
12: import "../resolvers/profiles/AddrResolver.sol";
13: import "./DNSClaimChecker.sol";
14: import "./PublicSuffixList.sol";
15: import "./IDNSRegistrar.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L5-L15

4: import "../../contracts/resolvers/profiles/IAddrResolver.sol";
5: import "../../contracts/resolvers/profiles/IExtendedResolver.sol";
6: import "../../contracts/resolvers/profiles/IExtendedDNSResolver.sol";
7: import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
8: import "../dnssec-oracle/BytesUtils.sol";
9: import "../dnssec-oracle/DNSSEC.sol";
10: import "../dnssec-oracle/RRUtils.sol";
11: import "../registry/ENSRegistry.sol";
12: import "../utils/HexUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L4-L12

5: import "./Owned.sol";
6: import "./BytesUtils.sol";
7: import "./RRUtils.sol";
8: import "./DNSSEC.sol";
9: import "./algorithms/Algorithm.sol";
10: import "./digests/Digest.sol";
11: import "@ensdomains/buffer/contracts/Buffer.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L5-L11

3: import "./Algorithm.sol";
4: import "../BytesUtils.sol";
5: import "./RSAVerify.sol";
6: import "@ensdomains/solsha1/contracts/SHA1.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L3-L6

3: import "./Digest.sol";
4: import "../BytesUtils.sol";
5: import "@ensdomains/solsha1/contracts/SHA1.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA1Digest.sol#L3-L5

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

Proof Of Concept

Several in-scope contracts.

See OffchainDNSResolver.sol for example.

Recommended Mitigation Steps

Include return parameters in NatSpec comments

Recommendation Code Style: (from Uniswap3)

    /// @notice Adds liquidity for the given recipient/tickLower/tickUpper position
    /// @dev The caller of this method receives a callback in the form of IUniswapV3MintCallback#uniswapV3MintCallback
    /// in which they must pay any token0 or token1 owed for the liquidity. The amount of token0/token1 due depends
    /// on tickLower, tickUpper, the amount of liquidity, and the current price.
    /// @param recipient The address for which the liquidity will be created
    /// @param tickLower The lower tick of the position in which to add liquidity
    /// @param tickUpper The upper tick of the position in which to add liquidity
    /// @param amount The amount of liquidity to mint
    /// @param data Any data that should be passed through to the callback
    /// @return amount0 The amount of token0 that was paid to mint the given amount of liquidity. Matches the value in the callback
    /// @return amount1 The amount of token1 that was paid to mint the given amount of liquidity. Matches the value in the callback
    function mint(
        address recipient,
        int24 tickLower,
        int24 tickUpper,
        uint128 amount,
        bytes calldata data
    ) external returns (uint256 amount0, uint256 amount1);

There is no need to initialize uint variables to zero as their default value is 0

Proof Of Concept

339: uint256 ret = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L339

59: uint256 count = 0;
72: uint256 constant RRSIG_TYPE = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L59

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L72

210: uint256 constant DNSKEY_FLAGS = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L210

236: uint256 constant DS_KEY_TAG = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L236

263: uint256 off = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L263

12: uint8 labelLength = 0;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L12

A check regarding whether the current value and the new value are the same should be added

Proof Of Concept

80: function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {
        suffixes = _suffixes;
        emit NewPublicSuffixList(address(suffixes));
    }

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

64: function setAlgorithm(uint8 id, Algorithm algo) public owner_only {
        algorithms[id] = algo;
        emit AlgorithmUpdated(id, address(algo));
    }

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L64

75: function setDigest(uint8 id, Digest digest) public owner_only {
        digests[id] = digest;
        emit DigestUpdated(id, address(digest));
    }

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L75

The test coverage rate of the project is 97%. Testing all functions is best practice in terms of security criteria. While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.

- How many contracts are in scope?:   21
- Total SLoC for these contracts?:  2103
...
- What is the overall line coverage percentage provided by your tests?:  90

Due to its capacity, test coverage is expected to be 100%.

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

Proof Of Concept

55: constructor(
        address _previousRegistrar,
        address _resolver,
        DNSSEC _dnssec,
        PublicSuffixList _suffixes,
        ENS _ens
    )

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L55

43: constructor(ENS _ens, DNSSEC _oracle, string memory _gatewayURL)

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L43

52: constructor(bytes memory _anchors)

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L52

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

Proof Of Concept

Several in-scope contracts.

See OffchainDNSResolver.sol for example.

Recommended Mitigation Steps

NatSpec comments should be increased in contracts

The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files

A good example:

import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {SafeCastLib} from "solmate/utils/SafeCastLib.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IProducer} from "src/interfaces/IProducer.sol";
import {GlobalState, UserState} from "src/Common.sol";

Proof Of Concept

4: import "../dnssec-oracle/DNSSEC.sol";
5: import "../dnssec-oracle/BytesUtils.sol";
6: import "../dnssec-oracle/RRUtils.sol";
7: import "../utils/HexUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L4-L7

7: import "../dnssec-oracle/BytesUtils.sol";
8: import "../dnssec-oracle/DNSSEC.sol";
9: import "../dnssec-oracle/RRUtils.sol";
10: import "../registry/ENSRegistry.sol";
11: import "../root/Root.sol";
12: import "../resolvers/profiles/AddrResolver.sol";
13: import "./DNSClaimChecker.sol";
14: import "./PublicSuffixList.sol";
15: import "./IDNSRegistrar.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L7-L15

4: import "../../contracts/resolvers/profiles/IAddrResolver.sol";
5: import "../../contracts/resolvers/profiles/IExtendedResolver.sol";
6: import "../../contracts/resolvers/profiles/IExtendedDNSResolver.sol";
8: import "../dnssec-oracle/BytesUtils.sol";
9: import "../dnssec-oracle/DNSSEC.sol";
10: import "../dnssec-oracle/RRUtils.sol";
11: import "../registry/ENSRegistry.sol";
12: import "../utils/HexUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L4

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L5

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L6

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L8

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L9

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L10

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L11

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L12

4: import "../dnssec-oracle/BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/RecordParser.sol#L4

5: import "./Owned.sol";
6: import "./BytesUtils.sol";
7: import "./RRUtils.sol";
8: import "./DNSSEC.sol";
9: import "./algorithms/Algorithm.sol";
10: import "./digests/Digest.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L5-L10

3: import "./BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L3

3: import "./Algorithm.sol";
4: import "./EllipticCurve.sol";
5: import "../BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L3-L5

3: import "./Algorithm.sol";
4: import "../BytesUtils.sol";
5: import "./RSAVerify.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L3-L5

3: import "./Algorithm.sol";
4: import "../BytesUtils.sol";
5: import "./RSAVerify.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA256Algorithm.sol#L3-L5

3: import "../BytesUtils.sol";
4: import "./ModexpPrecompile.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSAVerify.sol#L3-L4

3: import "./Digest.sol";
4: import "../BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA1Digest.sol#L3-L4

3: import "./Digest.sol";
4: import "../BytesUtils.sol";

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA256Digest.sol#L3-L4

Recommended Mitigation Steps

Use specific imports syntax per solidity docs recommendation.

0.8.12: string.concat() instead of abi.encodePacked(,)

0.8.13: Ability to use using for with a list of free functions

0.8.14:

ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.

0.8.15:

Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.

0.8.16:

Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.

0.8.17:

Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.

0.8.19: SMTChecker: New trusted mode that assumes that any compile-time available code is the actual used code, even in external calls. Bug Fixes:

  • Assembler: Avoid duplicating subassembly bytecode where possible.
  • Code Generator: Avoid including references to the deployed label of referenced functions if they are called right away.
  • ContractLevelChecker: Properly distinguish the case of missing base constructor arguments from having an unimplemented base function.
  • SMTChecker: Fix internal error caused by unhandled z3 expressions that come from the solver when bitwise operators are used.
  • SMTChecker: Fix internal error when using the custom NatSpec annotation to abstract free functions.
  • TypeChecker: Also allow external library functions in using for.

Proof Of Concept

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L2

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L3

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L2

pragma solidity ^0.8.11;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/RecordParser.sol#L2

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L2

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/RRUtils.sol#L1

pragma solidity >=0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/SHA1.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/EllipticCurve.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/ModexpPrecompile.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/P256SHA256Algorithm.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA1Algorithm.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSASHA256Algorithm.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/RSAVerify.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA1Digest.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/digests/SHA256Digest.sol#L1

pragma solidity ^0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/HexUtils.sol#L2

pragma solidity ^0.8.13;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L2

pragma solidity ~0.8.17;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L2

Recommended Mitigation Steps

Consider updating to a more recent solidity version.

Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters The following events should also add the previous original value in addition to the new value.

Proof Of Concept

53: event NewPublicSuffixList(address suffixes);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L53

Recommended Mitigation Steps

The events should include the new value and old value where possible.

An open TODO is present. It is recommended to avoid open TODOs as they may indicate programming errors that still need to be fixed.

Proof Of Concept

71: // TODO: More robust parsing that handles whitespace and multiple key/value pairs

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSClaimChecker.sol#L71

167: // TODO: Concatenate multiple text fields

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L167

291: // TODO: Check key isn't expired, unless updating key itself

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L291

There will be breaking changes in future versions of solidity, and at that point your code will no longer be compatable. While you may have the specific version to use in a configuration file, others that include your source files may not.

Proof Of Concept

1: pragma solidity >=0.8.4;

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/SHA1.sol#L1

Contracts are allowed to override their parents’ functions and change the visibility from external to public.

Proof Of Concept

function setPublicSuffixList(PublicSuffixList _suffixes) public onlyOwner {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L80

function proveAndClaimWithResolver(
        bytes memory name,
        DNSSEC.RRSetWithSignature[] memory input,
        address resolver,
        address addr
    ) public override {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L101

function setAlgorithm(uint8 id, Algorithm algo) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L64

function setDigest(uint8 id, Digest digest) public owner_only {

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/DNSSECImpl.sol#L75

Proof Of Concept

18: require(offset + len <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L18

196: require(idx + 2 <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L196

212: require(idx + 4 <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L212

228: require(idx + 32 <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L228

244: require(idx + 20 <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L244

265: require(len <= 32);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L265

266: require(idx + len <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L266

305: require(offset + len <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L305

337: require(len <= 52);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L337

343: require(char >= 0x30 && char <= 0x7A);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L343

345: require(decoded <= 0x20);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L345

373: revert();

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/BytesUtils.sol#L373

17: require(offset + len <= self.length);

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L17

Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))

Proof Of Concept

119: bytes32 node = keccak256(abi.encodePacked(rootNode, labelHash))
151: bytes32 node = keccak256(abi.encodePacked(parentNode, labelHash))
185: node = keccak256(abi.encodePacked(parentNode, label))

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L119

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L151

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/DNSRegistrar.sol#L185

223: abi.encodePacked(parentNode, name.keccak(idx, separator - idx))
            )

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnsregistrar/OffchainDNSResolver.sol#L223

12: bytes memory input = abi.encodePacked(
            uint256(base.length),
            uint256(exponent.length),
            uint256(modulus.length),
            base,
            exponent,
            modulus
        )

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/dnssec-oracle/algorithms/ModexpPrecompile.sol#L12

29: abi.encodePacked(
                            node,
                            bytesName.keccak(i + 1, labelLength)
                        )
                    )
46: abi.encodePacked(node, bytesName.keccak(0, labelLength))
        )

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L29

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/utils/NameEncoder.sol#L46

39: keccak256(abi.encodePacked(namehash(self, newOffset), labelhash))

https://github.com/code-423n4/2023-04-ens/tree/main/contracts/wrapper/BytesUtils.sol#L39

Recommended Mitigation Steps

Use bytes.concat() and upgrade to at least Solidity version 0.8.4 if required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment