Reclassify detectors (#331)

aderyn_core/src/detect/detector.rs

@@ -6,19 +6,19 @@ use crate::{
     context::workspace_context::WorkspaceContext,
     detect::{
         high::{
-            ArbitraryTransferFromDetector, BlockTimestampDeadlineDetector,
-            DelegateCallInLoopDetector,
+            ArbitraryTransferFromDetector, AvoidAbiEncodePackedDetector,
+            BlockTimestampDeadlineDetector, DelegateCallInLoopDetector,
+            UnprotectedInitializerDetector,
         },
         low::{
-            AvoidAbiEncodePackedDetector, CentralizationRiskDetector,
-            ConstantsInsteadOfLiteralsDetector, ContractsWithTodosDetector,
-            DeprecatedOZFunctionsDetector, EcrecoverDetector, EmptyBlockDetector,
-            InconsistentTypeNamesDetector, LargeLiteralValueDetector,
+            CentralizationRiskDetector, ConstantsInsteadOfLiteralsDetector,
+            ContractsWithTodosDetector, DeprecatedOZFunctionsDetector, EcrecoverDetector,
+            EmptyBlockDetector, InconsistentTypeNamesDetector, LargeLiteralValueDetector,
             NonReentrantBeforeOthersDetector, PushZeroOpcodeDetector, RequireWithStringDetector,
-            SolmateSafeTransferLibDetector, UnindexedEventsDetector,
-            UnprotectedInitializerDetector, UnsafeERC20FunctionsDetector, UnsafeERC721MintDetector,
-            UnspecificSolidityPragmaDetector, UselessInternalFunctionDetector,
-            UselessModifierDetector, UselessPublicFunctionDetector, ZeroAddressCheckDetector,
+            SolmateSafeTransferLibDetector, UnindexedEventsDetector, UnsafeERC20FunctionsDetector,
+            UnsafeERC721MintDetector, UnspecificSolidityPragmaDetector,
+            UselessInternalFunctionDetector, UselessModifierDetector,
+            UselessPublicFunctionDetector, ZeroAddressCheckDetector,
         },
     },
 };

aderyn_core/src/detect/low/avoid_abi_encode_packed.rs → aderyn_core/src/detect/high/avoid_abi_encode_packed.rs

@@ -58,7 +58,7 @@ impl IssueDetector for AvoidAbiEncodePackedDetector {
     }
 
     fn severity(&self) -> IssueSeverity {
-        IssueSeverity::Low
+        IssueSeverity::High
     }
 
     fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
@@ -92,7 +92,7 @@ mod avoid_abi_encode_packed_tests {
         // assert that the severity is low
         assert_eq!(
             detector.severity(),
-            crate::detect::detector::IssueSeverity::Low
+            crate::detect::detector::IssueSeverity::High
         );
         // assert that the title is correct
         assert_eq!(

aderyn_core/src/detect/high/mod.rs

@@ -1,7 +1,11 @@
 pub(crate) mod arbitrary_transfer_from;
+pub(crate) mod avoid_abi_encode_packed;
 pub(crate) mod block_timestamp_deadline;
 pub(crate) mod delegate_call_in_loop;
+pub(crate) mod unprotected_init_function;
 
 pub use arbitrary_transfer_from::ArbitraryTransferFromDetector;
+pub use avoid_abi_encode_packed::AvoidAbiEncodePackedDetector;
 pub use block_timestamp_deadline::BlockTimestampDeadlineDetector;
 pub use delegate_call_in_loop::DelegateCallInLoopDetector;
+pub use unprotected_init_function::UnprotectedInitializerDetector;

aderyn_core/src/detect/low/unprotected_init_function.rs → aderyn_core/src/detect/high/unprotected_init_function.rs

@@ -43,7 +43,7 @@ impl IssueDetector for UnprotectedInitializerDetector {
     }
 
     fn severity(&self) -> IssueSeverity {
-        IssueSeverity::Low
+        IssueSeverity::High
     }
 
     fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {

aderyn_core/src/detect/low/mod.rs

@@ -1,4 +1,3 @@
-pub(crate) mod avoid_abi_encode_packed;
 pub(crate) mod centralization_risk;
 pub(crate) mod constants_instead_of_literals;
 pub(crate) mod contracts_with_todos;
@@ -12,7 +11,6 @@ pub(crate) mod push_0_opcode;
 pub(crate) mod require_with_string;
 pub(crate) mod solmate_safe_transfer_lib;
 pub(crate) mod unindexed_events;
-pub(crate) mod unprotected_init_function;
 pub(crate) mod unsafe_erc20_functions;
 pub(crate) mod unsafe_oz_erc721_mint;
 pub(crate) mod unspecific_solidity_pragma;
@@ -21,7 +19,6 @@ pub(crate) mod useless_modifier;
 pub(crate) mod useless_public_function;
 pub(crate) mod zero_address_check;
 
-pub use avoid_abi_encode_packed::AvoidAbiEncodePackedDetector;
 pub use centralization_risk::CentralizationRiskDetector;
 pub use constants_instead_of_literals::ConstantsInsteadOfLiteralsDetector;
 pub use contracts_with_todos::ContractsWithTodosDetector;
@@ -35,7 +32,6 @@ pub use push_0_opcode::PushZeroOpcodeDetector;
 pub use require_with_string::RequireWithStringDetector;
 pub use solmate_safe_transfer_lib::SolmateSafeTransferLibDetector;
 pub use unindexed_events::UnindexedEventsDetector;
-pub use unprotected_init_function::UnprotectedInitializerDetector;
 pub use unsafe_erc20_functions::UnsafeERC20FunctionsDetector;
 pub use unsafe_oz_erc721_mint::UnsafeERC721MintDetector;
 pub use unspecific_solidity_pragma::UnspecificSolidityPragmaDetector;

report.json

@@ -176,6 +176,28 @@
         ],
         "title": "Using `delegatecall` in loop"
       },
+      {
+        "description": "Use `abi.encode()` instead which will pad items to 32 bytes, which will [prevent hash collisions](https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode) (e.g. `abi.encodePacked(0x123,0x456)` => `0x123456` => `abi.encodePacked(0x1,0x23456)`, but `abi.encode(0x123,0x456)` => `0x0...1230...456`). Unless there is a compelling reason, `abi.encode` should be preferred. If there is only one argument to `abi.encodePacked()` it can often be cast to `bytes()` or `bytes32()` [instead](https://ethereum.stackexchange.com/questions/30912/how-to-compare-strings-in-solidity#answer-82739).\nIf all arguments are strings and or bytes, `bytes.concat()` should be used instead.",
+        "detector_name": "avoid-abi-encode-packed",
+        "instances": [
+          {
+            "contract_path": "src/KeccakContract.sol",
+            "line_no": 18,
+            "src": "587:16"
+          },
+          {
+            "contract_path": "src/KeccakContract.sol",
+            "line_no": 22,
+            "src": "734:16"
+          },
+          {
+            "contract_path": "src/KeccakContract.sol",
+            "line_no": 26,
+            "src": "887:16"
+          }
+        ],
+        "title": "`abi.encodePacked()` should not be used with dynamic types when passing the result to a hash function such as `keccak256()`"
+      },
       {
         "description": "In the PoS model, proposers know well in advance if they will propose one or consecutive blocks ahead of time. In such a scenario, a malicious validator can hold back the transaction and execute it at a more favourable block number.Consider allowing function caller to specify swap deadline input parameter.",
         "detector_name": "block-timestamp-deadline",
@@ -304,12 +326,24 @@
           }
         ],
         "title": "Arbitrary `from` passed to `transferFrom` (or `safeTransferFrom`)"
+      },
+      {
+        "description": "Consider protecting the initializer functions with modifiers.",
+        "detector_name": "unprotected-initializer",
+        "instances": [
+          {
+            "contract_path": "src/UnprotectedInitialize.sol",
+            "line_no": 35,
+            "src": "820:33"
+          }
+        ],
+        "title": "Unprotected initializer"
       }
     ]
   },
   "issue_count": {
-    "high": 3,
-    "low": 22
+    "high": 5,
+    "low": 20
   },
   "low_issues": {
     "issues": [
@@ -377,28 +411,6 @@
         ],
         "title": "Solmate's SafeTransferLib does not check for token contract's existence"
       },
-      {
-        "description": "Use `abi.encode()` instead which will pad items to 32 bytes, which will [prevent hash collisions](https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode) (e.g. `abi.encodePacked(0x123,0x456)` => `0x123456` => `abi.encodePacked(0x1,0x23456)`, but `abi.encode(0x123,0x456)` => `0x0...1230...456`). Unless there is a compelling reason, `abi.encode` should be preferred. If there is only one argument to `abi.encodePacked()` it can often be cast to `bytes()` or `bytes32()` [instead](https://ethereum.stackexchange.com/questions/30912/how-to-compare-strings-in-solidity#answer-82739).\nIf all arguments are strings and or bytes, `bytes.concat()` should be used instead.",
-        "detector_name": "avoid-abi-encode-packed",
-        "instances": [
-          {
-            "contract_path": "src/KeccakContract.sol",
-            "line_no": 18,
-            "src": "587:16"
-          },
-          {
-            "contract_path": "src/KeccakContract.sol",
-            "line_no": 22,
-            "src": "734:16"
-          },
-          {
-            "contract_path": "src/KeccakContract.sol",
-            "line_no": 26,
-            "src": "887:16"
-          }
-        ],
-        "title": "`abi.encodePacked()` should not be used with dynamic types when passing the result to a hash function such as `keccak256()`"
-      },
       {
         "description": "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.",
         "detector_name": "ecrecover",
@@ -1179,18 +1191,6 @@
           }
         ],
         "title": "Inconsistency in declaring uint256/uint (or) int256/int variables within a contract"
-      },
-      {
-        "description": "Consider protecting the initializer functions with modifiers.",
-        "detector_name": "unprotected-initializer",
-        "instances": [
-          {
-            "contract_path": "src/UnprotectedInitialize.sol",
-            "line_no": 35,
-            "src": "820:33"
-          }
-        ],
-        "title": "Unprotected initializer"
       }
     ]
   }