ENS Contest - tnevler's results

Report

Low Risk

[L-1]: Missing checks for address(0x0)

Context:

1. previousRegistrar = _previousRegistrar; L62
2. resolver = _resolver; L63

Recommendation:

Add non-zero address checks when set address state variables.

[L-2]: Unsafe casting may overflow

Context:

1. return uint8(self[idx]); L183
2. decoded = uint8(base32HexTable[uint256(uint8(char)) - 0x30]); L344

Description:

While Solidity 0.8.x checks for overflows on arithmetic operations, it does not do so for casting.

Recommendation:

Use OpenZeppelin’s SafeCast library to prevent unexpected overflows.

[L-3]: setOwner should be two step process

Context:

1. import "./Owned.sol"; L5

Description:

Lack of two-step procedure for transfer of ownership makes it error-prone. It’s possible that the owner mistakenly transfers ownership to the uncontrolled account and it will break all functions with owner_only() modifier.

Recommendation:

Implement a two step process for transfer of ownership. Owner call transferOwnership() function where nominates an account. Nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This will confirm that the nominated EOA account is valid and active account.

[L-4]: The events should include the new value and old value where possible

Context:

1. emit NewPublicSuffixList(address(suffixes)); L82
2. emit AlgorithmUpdated(id, address(algo)); L66
3. emit DigestUpdated(id, address(digest)); L77

Description:

Events are generally emitted when sensitive changes are made to the contracts. Some events are missing important parameters.

Non-Critical Issues

[N-1]: Event is not used

Context:

event Debug(bytes32 x); L4

[N-2]: Function defines a named return variable but then it uses return statements

Context:

1. return ("", "", type(uint256).max); L25
2. return _enableNode(domain, 0); L171
3. return node; L204
4. return uint8(self[idx]); L183
5. return self.substring(offset, len); L46
6. return true; L129
7. return false; L131
8. return toAffinePoint(x1, y1, z1); L371
9. return verifyRRSet(input, block.timestamp); L96
10. return (proof, inception); L127
11. return rrset; L173
12. return (dnsName, node); L50

Recommendation:

Choose named return variable or return statement.

[N-3]: Follow Solidity standard naming conventions

Context:

1. uint256 otheroffset, L57 (Change to otherOffset)
2. uint256 otherlen L58 (Change to otherLen)
3. bytes constant base32HexTable = L322 (Constant name must be in capitalized SNAKE_CASE)
4. uint256 constant a = L21 (Constant name must be in capitalized SNAKE_CASE)
5. uint256 constant b = L23 (Constant name must be in capitalized SNAKE_CASE)
6. uint256 constant gx = L25 (Constant name must be in capitalized SNAKE_CASE)
7. uint256 constant gy = L27 (Constant name must be in capitalized SNAKE_CASE)
8. uint256 constant p = L29 (Constant name must be in capitalized SNAKE_CASE)
9. uint256 constant n = L31 (Constant name must be in capitalized SNAKE_CASE)
10. uint256 constant lowSmax = L34 (Constant name must be in capitalized SNAKE_CASE)
11. uint256 LHS = mulmod(y, y, p); // y^2 L142 (Variable name must be in mixedCase)
12. uint256 RHS = mulmod(mulmod(x, x, p), x, p); // x^3 L143 (Variable name must be in mixedCase)
13. uint256[2] memory Q L389 (function param name must be in mixedCase)
14. uint256[3] memory P = addAndReturnProjectivePoint(x1, y1, x2, y2); L408 (Variable name must be in mixedCase)
15. uint256 Px = inverseMod(P[2], p); L414 (Variable name must be in mixedCase)
16. bytes memory N, L15 (Function param name must be in mixedCase)
17. bytes memory E, L16 (Function param name must be in mixedCase)
18. bytes memory S L17 (Function param name must be in mixedCase)

Description:

The above codes don't follow Solidity's standard naming convention.

• Structs should be named using the CapWords style. Examples: MyCoin, Position, PositionXY.
• Events should be named using the CapWords style. Examples: Deposit, Transfer, Approval, BeforeTransfer, AfterTransfer.
• Functions should use mixedCase. Examples: getBalance, transfer, verifyOwner, addMember, changeOwner.
• Function arguments should use mixedCase. Examples: initialSupply, account, recipientAddress, senderAddress, newOwner.
• Local and State Variable should use mixedCase. Examples: totalSupply, remainingSupply, balancesOf, creatorAddress, isPreSale, tokenExchangeRate.
• Constants should be named with all capital letters with underscores separating words. Examples: MAX_BLOCKS, TOKEN_NAME, TOKEN_TICKER, CONTRACT_VERSION.
• Modifier should use mixedCase. Examples: onlyBy, onlyAfter, onlyDuringThePreSale.
• Enums, in the style of simple type declarations, should be named using the CapWords style. Examples: TokenGroup, Frame, HashStyle, CharacterLocation.

[N-4]: Missing leading underscores

Context:

1. uint16 constant CLASS_INET = 1; L16
2. uint16 constant TYPE_TXT = 16; L17
3. function parseRR( L136
4. function readTXT( L162
5. function parseAndResolve( L173
6. function resolveName( L190
7. function textNamehash( L209
8. function memcpy(uint256 dest, uint256 src, uint256 len) private pure { L273
9. function inverseMod(uint256 u, uint256 m) internal pure returns (uint256) { L40
10. function toProjectivePoint( L65
11. function addAndReturnProjectivePoint( L77
12. function toAffinePoint( L92
13. function zeroProj() L106
14. function zeroAffine() internal pure returns (uint256 x, uint256 y) { L117
15. function isZeroCurve( L124
16. function isOnCurve(uint256 x, uint256 y) internal pure returns (bool) { L137
17. function twiceProj( L159
18. function addProj( L208
19. function addProj2( L247
20. function add( L286
21. function twice( L302
22. function multiplyPowerBase2( L316
23. function multiplyScalar( L335
24. function multipleGeneratorByScalar( L377
25. function validateSignature( L386
26. function parseSignature( L30
27. function parseKey( L37
28. function validateSignedSet( L140
29. function validateRRs( L181
30. function verifySignature( L225
31. function verifyWithKnownKey( L254
32. function verifySignatureWithKey( L285
33. function verifyWithDS( L330
34. function verifyKeyWithDS( L373
35. function verifyDSHash( L415

Description:

Internal and private functions, state variables, constants, and immutables should starting with an underscore.

[N-5]: Mark visibility of variables

Context:

1. uint16 constant CLASS_INET = 1; L16
2. uint16 constant TYPE_TXT = 16; L17
3. bytes constant base32HexTable = L322
4. uint256 constant RRSIG_TYPE = 0; L72
5. uint256 constant RRSIG_ALGORITHM = 2; L73
6. uint256 constant RRSIG_LABELS = 3; L74
7. uint256 constant RRSIG_TTL = 4; L75
8. uint256 constant RRSIG_EXPIRATION = 8; L76
9. uint256 constant RRSIG_INCEPTION = 12; L77
10. uint256 constant RRSIG_KEY_TAG = 16; L78
11. uint256 constant RRSIG_SIGNER_NAME = 18; L79
12. uint256 constant DNSKEY_FLAGS = 0; L210
13. uint256 constant DNSKEY_PROTOCOL = 2; L211
14. uint256 constant DNSKEY_ALGORITHM = 3; L212
15. uint256 constant DNSKEY_PUBKEY = 4; L213
16. uint256 constant DS_KEY_TAG = 0; L236
17. uint256 constant DS_ALGORITHM = 2; L237
18. uint256 constant DS_DIGEST_TYPE = 3; L238
19. uint256 constant DS_DIGEST = 4; L239
20. uint256 constant a = L21
21. uint256 constant b = L23
22. uint256 constant gx = L25
23. uint256 constant gy = L27
24. uint256 constant p = L29
25. uint256 constant n = L31
26. uint256 constant lowSmax = L34
27. uint16 constant DNSCLASS_IN = 1; L27
28. uint16 constant DNSTYPE_DS = 43; L29
29. uint16 constant DNSTYPE_DNSKEY = 48; L30
30. uint256 constant DNSKEY_FLAG_ZONEKEY = 0x100; L32

[N-6]: Wrong order of Layout

Context:

1. bytes constant base32HexTable = L322 (constant can not go after internal function)
2. uint256 constant RRSIG_TYPE = 0; L72 (state variable can not go after internal function)

Description:

According to official solidity documentation inside each contract, library or interface, use the following order:

1. Type declarations
2. State variables
3. Events
4. Modifiers
5. Functions

[N-7]: NatSpec is missing

Context:

1. DNSClaimChecker.sol
2. OffchainDNSResolver.sol (6 functions are missing NatSpec)
3. DNSRegistrar.sol (5 functions are missing NatSpec)
4. function memcpy(uint256 dest, uint256 src, uint256 len) private pure { L273
5. function readSignedSet( L94
6. RRUtils.sol (7 functions are missing NatSpec)
7. RSASHA1Algorithm.sol
8. function parseSignature( L30
9. function parseKey( L37
10. RSASHA256Algorithm.sol
11. SHA1Digest.sol
12. SHA256Digest.sol
13. SHA1.sol
14. NameEncoder.sol

[N-8]: Typos

Context:

1. * @dev Compares a range of 'self' to all of 'other' and returns True iff L141 (Change iff to if)
2. * @return True iff the signature is valid. L15 (Change iff to if)

[N-9]: Natspec is incomplete

Context:

1. * @dev Compares two serial numbers using RFC1982 serial number math. L330 (param and return tags are missing)
2. `EllipticCurve.sol (param and return tags are missing in all functions)
3. * @dev Computes (base ^ exponent) % modulus over big numbers. L5 (param and return tags are missing)
4. RRUtils.SignedSet memory rrset, L227 (param rrset is missing)
5. bytes memory keyrdata, L287 (param keyrdata is missing)