adhoc-sol-files-report.md

Aderyn Analysis Report

This report was generated by Aderyn, a static analysis tool built by Cyfrin, a blockchain security company. This report is not a substitute for manual audit or security review. It should not be relied upon for any purpose other than to assist in the identification of potential security vulnerabilities.

Table of Contents

• Summary
  • Files Summary
  • Files Details
  • Issue Summary
• High Issues
  • H-1: Using delegatecall in loop
  • H-2: Uninitialized State Variables
  • H-3: Delegatecall made by the function without checks on any adress.
• Low Issues
  • L-1: Centralization Risk for trusted owners
  • L-2: ecrecover is susceptible to signature malleability
  • L-3: Solidity pragma should be specific, not wide
  • L-4: Missing checks for address(0) when assigning values to address state variables
  • L-5: public functions not used internally could be marked external
  • L-6: Define and use constant variables instead of using literals
  • L-7: Event is missing indexed fields
  • L-8: Empty require() / revert() statements
  • L-9: PUSH0 is not supported by all chains
  • L-10: Modifiers invoked only once can be shoe-horned into the function
  • L-11: Empty Block
  • L-12: Large literal values multiples of 10000 can be replaced with scientific notation
  • L-13: Internal functions called only once can be inlined
  • L-14: Contract still has TODOs
  • L-15: Inconsistency in declaring uint256/uint (or) int256/int variables within a contract. Use explicit size declarations (uint256 or int256).
  • L-16: Unused Custom Error

Summary

Files Summary

Key	Value
.sol Files	20
Total nSLOC	245

Files Details

Filepath	nSLOC
Counter.sol	20
DemoASTNodes.sol	31
Helper.sol	8
InconsistentUints.sol	17
InternalFunctions.sol	22
OnceModifierExample.sol	8
StateVariables.sol	58
inheritance/ExtendedInheritance.sol	17
inheritance/IContractInheritance.sol	4
inheritance/InheritanceBase.sol	8
multiple-versions/0.4/A.sol	5
multiple-versions/0.4/B.sol	5
multiple-versions/0.5/A.sol	5
multiple-versions/0.5/B.sol	7
multiple-versions/0.6/A.sol	5
multiple-versions/0.6/B.sol	5
multiple-versions/0.7/A.sol	5
multiple-versions/0.7/B.sol	5
multiple-versions/0.8/A.sol	5
multiple-versions/0.8/B.sol	5
Total	245

Issue Summary

Category	No. of Issues
High	3
Low	16

High Issues

H-1: Using delegatecall in loop

When calling delegatecall the same msg.value amount will be accredited multiple times.

1 Found Instances

• Found in inheritance/ExtendedInheritance.sol Line: 16

               target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i));

H-2: Uninitialized State Variables

Solidity does initialize variables by default when you declare them, however it's good practice to explicitly declare an initial value. For example, if you transfer money to an address we must make sure that the address has been initialized.

5 Found Instances

• Found in InconsistentUints.sol Line: 7

       int public intVariable; // 1

• Found in InconsistentUints.sol Line: 8

       int256 public int256Variable; // 1 

• Found in StateVariables.sol Line: 8

       uint256 private staticPrivateNumber;

• Found in StateVariables.sol Line: 9

       uint256 internal staticInternalNumber;

• Found in StateVariables.sol Line: 10

       uint256 public staticPublicNumber;

H-3: Delegatecall made by the function without checks on any adress.

Introduce checks on the address

1 Found Instances

• Found in inheritance/ExtendedInheritance.sol Line: 14

       function doSomethingElse(address target) external {

Low Issues

L-1: Centralization Risk for trusted owners

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

1 Found Instances

• Found in InternalFunctions.sol Line: 12

       function setValue(uint256 _newValue) external onlyOwner {

L-2: ecrecover is susceptible to signature malleability

The ecrecover function is susceptible to signature malleability. This means that the same message can be signed in multiple ways, allowing an attacker to change the message signature without invalidating it. This can lead to unexpected behavior in smart contracts, such as the loss of funds or the ability to bypass access control. Consider using OpenZeppelin's ECDSA library instead of the built-in function.

1 Found Instances

• Found in inheritance/ExtendedInheritance.sol Line: 21

           return ecrecover(theHash, v, r, s);

L-3: Solidity pragma should be specific, not wide

Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of pragma solidity ^0.8.0;, use pragma solidity 0.8.0;

16 Found Instances

• Found in Counter.sol Line: 2

   pragma solidity ^0.8.13;

• Found in DemoASTNodes.sol Line: 2

   pragma solidity >=0.8.0;

• Found in Helper.sol Line: 2

   pragma solidity >=0.8.0;

• Found in InconsistentUints.sol Line: 1

   pragma solidity ^0.8.24;

• Found in inheritance/IContractInheritance.sol Line: 2

   pragma solidity >=0.8.0;

• Found in inheritance/InheritanceBase.sol Line: 2

   pragma solidity ^0.8.0;

• Found in multiple-versions/0.4/A.sol Line: 2

   pragma solidity ^0.4.0;

• Found in multiple-versions/0.4/B.sol Line: 2

   pragma solidity ^0.4.0;

• Found in multiple-versions/0.5/A.sol Line: 2

   pragma solidity ^0.5.0;

• Found in multiple-versions/0.5/B.sol Line: 2

   pragma solidity ^0.5.0;

• Found in multiple-versions/0.6/A.sol Line: 2

   pragma solidity ^0.6.0;

• Found in multiple-versions/0.6/B.sol Line: 2

   pragma solidity ^0.6.0;

• Found in multiple-versions/0.7/A.sol Line: 2

   pragma solidity ^0.7.0;

• Found in multiple-versions/0.7/B.sol Line: 2

   pragma solidity ^0.7.0;

• Found in multiple-versions/0.8/A.sol Line: 2

   pragma solidity ^0.8.0;

• Found in multiple-versions/0.8/B.sol Line: 2

   pragma solidity ^0.8.0;

L-4: Missing checks for address(0) when assigning values to address state variables

Check for address(0) when assigning values to address state variables.

1 Found Instances

• Found in StateVariables.sol Line: 58

           addr = newAddr;

L-5: public functions not used internally could be marked external

Instead of marking a function as public, consider marking it as external if it is not used internally.

6 Found Instances

• Found in Counter.sol Line: 7

       function setNumber(uint256 newNumber) public {

• Found in StateVariables.sol Line: 47

       function setAddrNoZeroError(address newAddr) public {

• Found in StateVariables.sol Line: 52

       function setAddrNoZeroRequire(address newAddr) public {

• Found in StateVariables.sol Line: 57

       function setAddrNoCheck(address newAddr) public {

• Found in StateVariables.sol Line: 61

       function setEmptyAlteredNumbers(

• Found in StateVariables.sol Line: 71

       function setNonEmptyAlteredNumbers(

L-6: Define and use constant variables instead of using literals

If the same constant literal value is used multiple times, create a constant state variable and reference it throughout the contract.

6 Found Instances

• Found in DemoASTNodes.sol Line: 15

               if (i == 3) {

• Found in DemoASTNodes.sol Line: 20

               if (i == 5) {

• Found in DemoASTNodes.sol Line: 31

           uint256[] memory numbers = new uint256[](5);

• Found in DemoASTNodes.sol Line: 35

           numbers[3] = 3;

• Found in DemoASTNodes.sol Line: 36

           numbers[4] = 4;

• Found in DemoASTNodes.sol Line: 38

           int256 i = 4; 

L-7: Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

2 Found Instances

• Found in inheritance/ExtendedInheritance.sol Line: 7

       event DoSomethingElse(uint256 somethingElse);

• Found in inheritance/InheritanceBase.sol Line: 7

       event Do(uint256 something);

L-8: Empty require() / revert() statements

Use descriptive reason strings or custom errors for revert paths.

1 Found Instances

• Found in DemoASTNodes.sol Line: 7

           require(to != address(0));

L-9: PUSH0 is not supported by all chains

Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail.

10 Found Instances

• Found in Counter.sol Line: 2

   pragma solidity ^0.8.13;

• Found in DemoASTNodes.sol Line: 2

   pragma solidity >=0.8.0;

• Found in Helper.sol Line: 2

   pragma solidity >=0.8.0;

• Found in InconsistentUints.sol Line: 1

   pragma solidity ^0.8.24;

• Found in StateVariables.sol Line: 2

   pragma solidity 0.8.20;

• Found in inheritance/ExtendedInheritance.sol Line: 2

   pragma solidity 0.8.20;

• Found in inheritance/IContractInheritance.sol Line: 2

   pragma solidity >=0.8.0;

• Found in inheritance/InheritanceBase.sol Line: 2

   pragma solidity ^0.8.0;

• Found in multiple-versions/0.8/A.sol Line: 2

   pragma solidity ^0.8.0;

• Found in multiple-versions/0.8/B.sol Line: 2

   pragma solidity ^0.8.0;

L-10: Modifiers invoked only once can be shoe-horned into the function

3 Found Instances

• Found in DemoASTNodes.sol Line: 6

       modifier iHaveAPlaceholder(address to) {

• Found in InternalFunctions.sol Line: 18

       modifier onlyOwner() {

• Found in OnceModifierExample.sol Line: 6

       modifier onlyOnce() {

L-11: Empty Block

Consider removing empty blocks.

2 Found Instances

• Found in OnceModifierExample.sol Line: 10

       function perform() external onlyOnce {

• Found in multiple-versions/0.5/B.sol Line: 8

       function emptyBlockCatchMe() external {

L-12: Large literal values multiples of 10000 can be replaced with scientific notation

Use e notation, for example: 1e18, instead of its full numeric value.

1 Found Instances

• Found in DemoASTNodes.sol Line: 14

           for (uint256 i = 0; i < 10000000; ++i) {

L-13: Internal functions called only once can be inlined

Instead of separating the logic into a separate function, consider inlining the logic into the calling function. This can reduce the number of function calls and improve readability.

1 Found Instances

• Found in InternalFunctions.sol Line: 28

       function internalSet2(uint256 _newValue) internal {

L-14: Contract still has TODOs

Contract contains comments with TODOS

1 Found Instances

• Found in Counter.sol Line: 4

   contract Counter {

L-15: Inconsistency in declaring uint256/uint (or) int256/int variables within a contract. Use explicit size declarations (uint256 or int256).

Consider keeping the naming convention consistent in a given contract. Explicit size declarations are preferred (uint256, int256) over implicit ones (uint, int) to avoid confusion.

7 Found Instances

• Found in InconsistentUints.sol Line: 5

       uint public uintVariable; // 1

• Found in InconsistentUints.sol Line: 7

       int public intVariable; // 1

• Found in InconsistentUints.sol Line: 11

           uint personUint; // 2

• Found in InconsistentUints.sol Line: 12

           mapping (uint => uint256) personMap; // 3 2

• Found in InconsistentUints.sol Line: 15

       uint[] public uintArray; // 4

• Found in InconsistentUints.sol Line: 16

       mapping(uint256 => uint other) u2uMapping; // 5 3

• Found in InconsistentUints.sol Line: 19

       constructor(uint _uintInitial, uint256 _uint256Initial) { // 6 4

L-16: Unused Custom Error

it is recommended that the definition be removed when custom error is unused

1 Found Instances

• Found in Helper.sol Line: 8

   error NotNice();