From 61295dbb1547743ea667e02fbdc432b2404e4368 Mon Sep 17 00:00:00 2001 From: Alex Roan Date: Tue, 6 Aug 2024 12:59:09 +0100 Subject: [PATCH] Revert "Detector: Uninitialized local variables" (#658) --- aderyn_core/src/context/meta_workspace.rs | 4 - aderyn_core/src/context/workspace_context.rs | 1 - aderyn_core/src/detect/detector.rs | 5 - aderyn_core/src/detect/low/mod.rs | 2 - .../low/uninitialized_local_variables.rs | 142 --- aderyn_core/src/visitor/workspace_visitor.rs | 13 - reports/report.json | 426 +-------- reports/report.md | 333 +------ reports/report.sarif | 881 ++---------------- reports/templegold-report.md | 92 +- .../src/UninitializedLocalVariables.sol | 117 --- 11 files changed, 82 insertions(+), 1934 deletions(-) delete mode 100644 aderyn_core/src/detect/low/uninitialized_local_variables.rs delete mode 100644 tests/contract-playground/src/UninitializedLocalVariables.sol diff --git a/aderyn_core/src/context/meta_workspace.rs b/aderyn_core/src/context/meta_workspace.rs index 98792b9f3..cf1f34d8b 100644 --- a/aderyn_core/src/context/meta_workspace.rs +++ b/aderyn_core/src/context/meta_workspace.rs @@ -188,10 +188,6 @@ impl WorkspaceContext { self.yul_identifiers_context.keys().collect() } - pub fn yul_assignments(&self) -> Vec<&YulAssignment> { - self.yul_assignments_context.keys().collect() - } - pub fn yul_literals(&self) -> Vec<&YulLiteral> { self.yul_literals_context.keys().collect() } diff --git a/aderyn_core/src/context/workspace_context.rs b/aderyn_core/src/context/workspace_context.rs index 4d4759933..5cd2cdb47 100644 --- a/aderyn_core/src/context/workspace_context.rs +++ b/aderyn_core/src/context/workspace_context.rs @@ -94,7 +94,6 @@ pub struct WorkspaceContext { pub(crate) yul_function_calls_context: HashMap, pub(crate) yul_identifiers_context: HashMap, pub(crate) yul_literals_context: HashMap, - pub(crate) yul_assignments_context: HashMap, } impl WorkspaceContext { diff --git a/aderyn_core/src/detect/detector.rs b/aderyn_core/src/detect/detector.rs index 1deda2be1..3005acdb4 100644 --- a/aderyn_core/src/detect/detector.rs +++ b/aderyn_core/src/detect/detector.rs @@ -80,7 +80,6 @@ pub fn get_all_issue_detectors() -> Vec> { Box::::default(), Box::::default(), Box::::default(), - Box::::default(), Box::::default(), ] } @@ -158,7 +157,6 @@ pub(crate) enum IssueDetectorNamePool { TxOriginUsedForAuth, MsgValueInLoop, ContractLocksEther, - UninitializedLocalVariable, ReturnBomb, // NOTE: `Undecided` will be the default name (for new bots). // If it's accepted, a new variant will be added to this enum before normalizing it in aderyn @@ -169,9 +167,6 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option { - Some(Box::::default()) - } IssueDetectorNamePool::ReturnBomb => Some(Box::::default()), IssueDetectorNamePool::UnusedStateVariable => { Some(Box::::default()) diff --git a/aderyn_core/src/detect/low/mod.rs b/aderyn_core/src/detect/low/mod.rs index 13c3fd466..bebd9f346 100644 --- a/aderyn_core/src/detect/low/mod.rs +++ b/aderyn_core/src/detect/low/mod.rs @@ -18,7 +18,6 @@ pub(crate) mod return_bomb; pub(crate) mod reverts_and_requries_in_loops; pub(crate) mod solmate_safe_transfer_lib; pub(crate) mod unindexed_events; -pub(crate) mod uninitialized_local_variables; pub(crate) mod unsafe_erc20_functions; pub(crate) mod unsafe_oz_erc721_mint; pub(crate) mod unspecific_solidity_pragma; @@ -49,7 +48,6 @@ pub use return_bomb::ReturnBombDetector; pub use reverts_and_requries_in_loops::RevertsAndRequiresInLoopsDetector; pub use solmate_safe_transfer_lib::SolmateSafeTransferLibDetector; pub use unindexed_events::UnindexedEventsDetector; -pub use uninitialized_local_variables::UninitializedLocalVariableDetector; pub use unsafe_erc20_functions::UnsafeERC20FunctionsDetector; pub use unsafe_oz_erc721_mint::UnsafeERC721MintDetector; pub use unspecific_solidity_pragma::UnspecificSolidityPragmaDetector; diff --git a/aderyn_core/src/detect/low/uninitialized_local_variables.rs b/aderyn_core/src/detect/low/uninitialized_local_variables.rs deleted file mode 100644 index 9b775da35..000000000 --- a/aderyn_core/src/detect/low/uninitialized_local_variables.rs +++ /dev/null @@ -1,142 +0,0 @@ -use std::collections::{BTreeMap, HashSet}; -use std::error::Error; - -use crate::ast::{ASTNode, NodeID}; - -use crate::capture; -use crate::context::browser::ExtractReferencedDeclarations; -use crate::detect::detector::IssueDetectorNamePool; -use crate::{ - context::workspace_context::WorkspaceContext, - detect::detector::{IssueDetector, IssueSeverity}, -}; -use eyre::Result; - -#[derive(Default)] -pub struct UninitializedLocalVariableDetector { - // Keys are: [0] source file name, [1] line number, [2] character location of node. - // Do not add items manually, use `capture!` to add nodes to this BTreeMap. - found_instances: BTreeMap<(String, usize, String), NodeID>, -} - -impl IssueDetector for UninitializedLocalVariableDetector { - fn detect(&mut self, context: &WorkspaceContext) -> Result> { - // Assumption: - // VariableDeclarationStatements consists of statements that look like `uint x;` `uint y, z;`, `uint p = 12;` - // but are not declared at the contract level (state level) but rather within functions and modifiers - - let mut potentially_uninitialized_local_variables = HashSet::new(); - - for variable_declaration_statement in context - .variable_declaration_statements() - .into_iter() - .filter(|s| s.initial_value.is_none()) - { - potentially_uninitialized_local_variables.extend( - variable_declaration_statement - .declarations - .iter() - .flat_map(|s| { - if let Some(ref s) = s { - return Some(s.id); - } - None - }), - ); - } - - // We can filter out the initialized variables by looking at LHS of assignments. - // This trick works for local variables because it's not possible to have structs, mappings, dynamic arrays - // declared local to the function. - for assignment in context.assignments() { - let references = - ExtractReferencedDeclarations::from(assignment.left_hand_side.as_ref()).extracted; - potentially_uninitialized_local_variables.retain(|v| !references.contains(v)); - } - - // Blacklist variables assigned via Yul Assignments - let mut blacklist_variable_names = HashSet::new(); - - for yul_assignment in context.yul_assignments() { - blacklist_variable_names - .extend(yul_assignment.variable_names.iter().map(|v| v.name.clone())) - } - - for id in potentially_uninitialized_local_variables { - if let Some(ASTNode::VariableDeclaration(v)) = context.nodes.get(&id) { - if !blacklist_variable_names.contains(&v.name) { - // Ignore memory structs because they can have an initializeMethod of their own. So not covered under the assignment operator - if v.type_descriptions - .type_string - .as_ref() - .is_some_and(|type_string| !type_string.contains("struct ")) - { - capture!(self, context, v); - } - } - } - } - - Ok(!self.found_instances.is_empty()) - } - - fn severity(&self) -> IssueSeverity { - IssueSeverity::Low - } - - fn title(&self) -> String { - String::from("Uninitialized local variables.") - } - - fn description(&self) -> String { - String::from("Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability.") - } - - fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { - self.found_instances.clone() - } - - fn name(&self) -> String { - format!("{}", IssueDetectorNamePool::UninitializedLocalVariable) - } -} - -#[cfg(test)] -mod uninitialized_local_variables_detector_tests { - use serial_test::serial; - - use crate::detect::{ - detector::IssueDetector, - low::uninitialized_local_variables::UninitializedLocalVariableDetector, - }; - - #[test] - #[serial] - fn test_uninitialized_local_variables() { - let context = crate::detect::test_utils::load_solidity_source_unit( - "../tests/contract-playground/src/UninitializedLocalVariables.sol", - ); - - let mut detector = UninitializedLocalVariableDetector::default(); - let found = detector.detect(&context).unwrap(); - - println!( - "Line numbers of uninitialized local variables: {:?}", - detector - .instances() - .into_iter() - .map(|(i, _)| i.1) - .collect::>() - ); - - // assert that the detector found an issue - assert!(found); - // assert that the detector found the correct number of instances - assert_eq!(detector.instances().len(), 12); - // assert the severity is low - assert_eq!( - detector.severity(), - crate::detect::detector::IssueSeverity::Low - ); - } -} diff --git a/aderyn_core/src/visitor/workspace_visitor.rs b/aderyn_core/src/visitor/workspace_visitor.rs index 533273405..b75d4bd4f 100644 --- a/aderyn_core/src/visitor/workspace_visitor.rs +++ b/aderyn_core/src/visitor/workspace_visitor.rs @@ -103,19 +103,6 @@ impl ASTConstVisitor for WorkspaceContext { Ok(true) } - fn visit_yul_assignment(&mut self, node: &YulAssignment) -> Result { - self.yul_assignments_context.insert( - node.clone(), - NodeContext { - source_unit_id: self.last_source_unit_id, - contract_definition_id: self.last_contract_definition_id, - function_definition_id: self.last_function_definition_id, - modifier_definition_id: self.last_modifier_definition_id, - }, - ); - Ok(true) - } - // Read the following like follows - generate_visit_methods_for_workspace_context_with_insert_node! { // Explanation for the 1st one : Create a method called `visit_assignment` that takes in `Assignment` as parameter and puts it inside `assignments_context` diff --git a/reports/report.json b/reports/report.json index 83fa44e05..7708d11f9 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 82, - "total_sloc": 2422 + "total_source_units": 81, + "total_sloc": 2334 }, "files_details": { "files_details": [ @@ -217,10 +217,6 @@ "file_path": "src/UncheckedSend.sol", "n_sloc": 18 }, - { - "file_path": "src/UninitializedLocalVariables.sol", - "n_sloc": 88 - }, { "file_path": "src/UninitializedStateVariable.sol", "n_sloc": 29 @@ -337,7 +333,7 @@ }, "issue_count": { "high": 36, - "low": 30 + "low": 29 }, "high_issues": { "issues": [ @@ -376,30 +372,6 @@ "line_no": 26, "src": "887:16", "src_char": "887:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 45, - "src": "1994:16", - "src_char": "1994:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 53, - "src": "2692:16", - "src_char": "2692:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 102, - "src": "4694:16", - "src_char": "4694:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 110, - "src": "5230:16", - "src_char": "5230:16" } ] }, @@ -1405,12 +1377,6 @@ "src": "145:9", "src_char": "145:9" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 6, - "src": "95:12", - "src_char": "95:12" - }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 7, @@ -1607,18 +1573,6 @@ "line_no": 75, "src": "1691:18", "src_char": "1691:18" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 98, - "src": "4375:20", - "src_char": "4375:20" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 106, - "src": "4853:27", - "src_char": "4853:27" } ] }, @@ -2407,12 +2361,6 @@ "src": "32:23", "src_char": "32:23" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 2, - "src": "33:23", - "src_char": "33:23" - }, { "contract_path": "src/UnusedStateVariables.sol", "line_no": 2, @@ -2625,18 +2573,6 @@ "src": "2539:25", "src_char": "2539:25" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 8, - "src": "124:16", - "src_char": "124:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 61, - "src": "3093:17", - "src_char": "3093:17" - }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 17, @@ -2962,48 +2898,6 @@ "src": "1190:7", "src_char": "1190:7" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 33, - "src": "1204:42", - "src_char": "1204:42" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 85, - "src": "3846:42", - "src_char": "3846:42" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 89, - "src": "4027:2", - "src_char": "4027:2" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 91, - "src": "4101:42", - "src_char": "4101:42" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 92, - "src": "4175:2", - "src_char": "4175:2" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 99, - "src": "4444:42", - "src_char": "4444:42" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 107, - "src": "4942:42", - "src_char": "4942:42" - }, { "contract_path": "src/WeakRandomness.sol", "line_no": 25, @@ -3417,12 +3311,6 @@ "src": "32:24", "src_char": "32:24" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 2, - "src": "33:23", - "src_char": "33:23" - }, { "contract_path": "src/UnsafeERC721Mint.sol", "line_no": 2, @@ -4095,66 +3983,6 @@ "src": "133:6", "src_char": "133:6" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 10, - "src": "211:17", - "src_char": "211:17" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 16, - "src": "434:22", - "src_char": "434:22" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 25, - "src": "770:15", - "src_char": "770:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 31, - "src": "1062:20", - "src_char": "1062:20" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 40, - "src": "1538:3", - "src_char": "1538:3" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 48, - "src": "2110:8", - "src_char": "2110:8" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 63, - "src": "3205:11", - "src_char": "3205:11" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 69, - "src": "3392:16", - "src_char": "3392:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 97, - "src": "4325:3", - "src_char": "4325:3" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 105, - "src": "4785:8", - "src_char": "4785:8" - }, { "contract_path": "src/eth2/DepositContract.sol", "line_no": 59, @@ -4332,150 +4160,6 @@ "line_no": 14, "src": "377:4", "src_char": "377:4" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 56, - "src": "2871:3", - "src_char": "2871:3" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 56, - "src": "2876:11", - "src_char": "2876:11" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 56, - "src": "2889:10", - "src_char": "2889:10" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 56, - "src": "2901:10", - "src_char": "2901:10" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 56, - "src": "2913:15", - "src_char": "2913:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 56, - "src": "2930:14", - "src_char": "2930:14" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 57, - "src": "2958:8", - "src_char": "2958:8" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 57, - "src": "2968:16", - "src_char": "2968:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 57, - "src": "2986:15", - "src_char": "2986:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 57, - "src": "3003:15", - "src_char": "3003:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 57, - "src": "3020:20", - "src_char": "3020:20" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 57, - "src": "3042:19", - "src_char": "3042:19" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 113, - "src": "5369:3", - "src_char": "5369:3" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 113, - "src": "5374:11", - "src_char": "5374:11" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 113, - "src": "5387:10", - "src_char": "5387:10" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 113, - "src": "5399:10", - "src_char": "5399:10" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 113, - "src": "5411:15", - "src_char": "5411:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 113, - "src": "5428:14", - "src_char": "5428:14" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 114, - "src": "5452:8", - "src_char": "5452:8" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 114, - "src": "5462:16", - "src_char": "5462:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 114, - "src": "5480:15", - "src_char": "5480:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 114, - "src": "5497:15", - "src_char": "5497:15" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 114, - "src": "5514:20", - "src_char": "5514:20" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 114, - "src": "5536:19", - "src_char": "5536:19" } ] }, @@ -4575,12 +4259,6 @@ "src": "145:9", "src_char": "145:9" }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 6, - "src": "95:12", - "src_char": "95:12" - }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 13, @@ -4729,103 +4407,6 @@ } ] }, - { - "title": "Uninitialized local variables.", - "description": "Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability.", - "detector_name": "uninitialized-local-variable", - "instances": [ - { - "contract_path": "src/ConstantFuncsAssembly.sol", - "line_no": 18, - "src": "478:14", - "src_char": "478:14" - }, - { - "contract_path": "src/ConstantFuncsAssembly.sol", - "line_no": 27, - "src": "716:14", - "src_char": "716:14" - }, - { - "contract_path": "src/StorageParameters.sol", - "line_no": 8, - "src": "184:11", - "src_char": "184:11" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 10, - "src": "211:17", - "src_char": "211:17" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 11, - "src": "243:17", - "src_char": "243:17" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 12, - "src": "278:20", - "src_char": "278:20" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 13, - "src": "312:16", - "src_char": "312:16" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 14, - "src": "346:20", - "src_char": "346:20" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 15, - "src": "390:19", - "src_char": "390:19" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 16, - "src": "434:22", - "src_char": "434:22" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 17, - "src": "481:22", - "src_char": "481:22" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 18, - "src": "531:25", - "src_char": "531:25" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 19, - "src": "580:21", - "src_char": "580:21" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 20, - "src": "629:25", - "src_char": "629:25" - }, - { - "contract_path": "src/UninitializedLocalVariables.sol", - "line_no": 21, - "src": "681:24", - "src_char": "681:24" - } - ] - }, { "title": "Return Bomb", "description": "A low level callee may consume all callers gas unexpectedly. Avoid unlimited implicit decoding of returndata on calls to unchecked addresses. You can limit the gas by passing a gas limit as an option to the call. For example, `unknownAdress.call{gas: gasLimitHere}(\"calldata\")` That would act as a safety net from OOG errors.\n ", @@ -4906,7 +4487,6 @@ "tx-origin-used-for-auth", "msg-value-in-loop", "contract-locks-ether", - "uninitialized-local-variable", "return-bomb" ] } \ No newline at end of file diff --git a/reports/report.md b/reports/report.md index 06029c102..f335c32fc 100644 --- a/reports/report.md +++ b/reports/report.md @@ -73,8 +73,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-26: Potentially unused `private` / `internal` state variables found.](#l-26-potentially-unused-private--internal-state-variables-found) - [L-27: Functions declared `pure` / `view` but contains assembly](#l-27-functions-declared-pure--view-but-contains-assembly) - [L-28: Boolean equality is not required.](#l-28-boolean-equality-is-not-required) - - [L-29: Uninitialized local variables.](#l-29-uninitialized-local-variables) - - [L-30: Return Bomb](#l-30-return-bomb) + - [L-29: Return Bomb](#l-29-return-bomb) # Summary @@ -83,8 +82,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 82 | -| Total nSLOC | 2422 | +| .sol Files | 81 | +| Total nSLOC | 2334 | ## Files Details @@ -144,7 +143,6 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/TxOriginUsedForAuth.sol | 42 | | src/UncheckedReturn.sol | 33 | | src/UncheckedSend.sol | 18 | -| src/UninitializedLocalVariables.sol | 88 | | src/UninitializedStateVariable.sol | 29 | | src/UnprotectedInitialize.sol | 25 | | src/UnsafeERC721Mint.sol | 18 | @@ -173,7 +171,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/reused_contract_name/ContractB.sol | 7 | | src/uniswap/UniswapV2Swapper.sol | 50 | | src/uniswap/UniswapV3Swapper.sol | 150 | -| **Total** | **2422** | +| **Total** | **2334** | ## Issue Summary @@ -181,7 +179,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 36 | -| Low | 30 | +| Low | 29 | # High Issues @@ -208,7 +206,7 @@ When calling `delegatecall` the same `msg.value` amount will be accredited multi 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). If all arguments are strings and or bytes, `bytes.concat()` should be used instead. -
7 Found Instances +
3 Found Instances - Found in src/KeccakContract.sol [Line: 18](../tests/contract-playground/src/KeccakContract.sol#L18) @@ -229,30 +227,6 @@ If all arguments are strings and or bytes, `bytes.concat()` should be used inste return keccak256(abi.encodePacked(a, b)); ``` -- Found in src/UninitializedLocalVariables.sol [Line: 45](../tests/contract-playground/src/UninitializedLocalVariables.sol#L45) - - ```solidity - string memory combinedString = string(abi.encodePacked(initializedString, uninitializedString)); - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 53](../tests/contract-playground/src/UninitializedLocalVariables.sol#L53) - - ```solidity - string memory arrayCombinedString = string(abi.encodePacked(initializedStringArray[0], uninitializedStringArray[0])); - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 102](../tests/contract-playground/src/UninitializedLocalVariables.sol#L102) - - ```solidity - string memory combinedString = string(abi.encodePacked(delayedString, " now")); - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 110](../tests/contract-playground/src/UninitializedLocalVariables.sol#L110) - - ```solidity - string memory arrayCombinedString = string(abi.encodePacked(delayedStringArray[0], " elements")); - ``` -
@@ -1227,7 +1201,7 @@ If the length of a dynamic array (storage variable) directly assigned to, it may 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. -
24 Found Instances +
23 Found Instances - Found in src/AssemblyExample.sol [Line: 5](../tests/contract-playground/src/AssemblyExample.sol#L5) @@ -1314,12 +1288,6 @@ Solidity does initialize variables by default when you declare them, however it' uint256 y; ``` -- Found in src/UninitializedLocalVariables.sol [Line: 6](../tests/contract-playground/src/UninitializedLocalVariables.sol#L6) - - ```solidity - uint256 stateVarUint; - ``` - - Found in src/UninitializedStateVariable.sol [Line: 7](../tests/contract-playground/src/UninitializedStateVariable.sol#L7) ```solidity @@ -1474,7 +1442,7 @@ The transaction `address(payable?).send(address)` may fail because of reasons li The patterns `if (… || true)` and `if (.. && false)` will always evaluate to true and false respectively. -
12 Found Instances +
10 Found Instances - Found in src/MisusedBoolean.sol [Line: 12](../tests/contract-playground/src/MisusedBoolean.sol#L12) @@ -1537,18 +1505,6 @@ The patterns `if (… || true)` and `if (.. && false)` will always evaluate to t if (isEven(num) && !NO) { ``` -- Found in src/UninitializedLocalVariables.sol [Line: 98](../tests/contract-playground/src/UninitializedLocalVariables.sol#L98) - - ```solidity - bool conjunction = delayedBool && false; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 106](../tests/contract-playground/src/UninitializedLocalVariables.sol#L106) - - ```solidity - bool arrayConjunction = delayedBoolArray[0] && true; - ``` -
@@ -2316,7 +2272,7 @@ ERC20 functions may not behave as expected. For example: return values are not a 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;` -
26 Found Instances +
25 Found Instances - Found in src/CompilerBugStorageSignedIntegerArray.sol [Line: 2](../tests/contract-playground/src/CompilerBugStorageSignedIntegerArray.sol#L2) @@ -2421,12 +2377,6 @@ Consider using a specific version of Solidity in your contracts instead of a wid pragma solidity ^0.7.0; ``` -- Found in src/UninitializedLocalVariables.sol [Line: 2](../tests/contract-playground/src/UninitializedLocalVariables.sol#L2) - - ```solidity - pragma solidity ^0.8.0; - ``` - - Found in src/UnusedStateVariables.sol [Line: 2](../tests/contract-playground/src/UnusedStateVariables.sol#L2) ```solidity @@ -2530,7 +2480,7 @@ Check for `address(0)` when assigning values to address state variables. Instead of marking a function as `public`, consider marking it as `external` if it is not used internally. -
35 Found Instances +
33 Found Instances - Found in src/ArbitraryTransferFrom.sol [Line: 28](../tests/contract-playground/src/ArbitraryTransferFrom.sol#L28) @@ -2647,18 +2597,6 @@ Instead of marking a function as `public`, consider marking it as `external` if function setNonEmptyAlteredNumbers( ``` -- Found in src/UninitializedLocalVariables.sol [Line: 8](../tests/contract-playground/src/UninitializedLocalVariables.sol#L8) - - ```solidity - function testAllDataTypes() public pure { - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 61](../tests/contract-playground/src/UninitializedLocalVariables.sol#L61) - - ```solidity - function testAllDataTypes2() public pure { - ``` - - Found in src/UninitializedStateVariable.sol [Line: 17](../tests/contract-playground/src/UninitializedStateVariable.sol#L17) ```solidity @@ -2751,7 +2689,7 @@ Instead of marking a function as `public`, consider marking it as `external` if If the same constant literal value is used multiple times, create a constant state variable and reference it throughout the contract. -
51 Found Instances +
44 Found Instances - Found in src/BooleanEquality.sol [Line: 6](../tests/contract-playground/src/BooleanEquality.sol#L6) @@ -2982,48 +2920,6 @@ If the same constant literal value is used multiple times, create a constant sta if (UncheckedHelperExternal(address(0x12345)).two() != 2) { ``` -- Found in src/UninitializedLocalVariables.sol [Line: 33](../tests/contract-playground/src/UninitializedLocalVariables.sol#L33) - - ```solidity - address[1] memory initializedAddressArray = [0x0000000000000000000000000000000000000001]; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 85](../tests/contract-playground/src/UninitializedLocalVariables.sol#L85) - - ```solidity - delayedAddress = 0x0000000000000000000000000000000000000001; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 89](../tests/contract-playground/src/UninitializedLocalVariables.sol#L89) - - ```solidity - delayedUintArray[0] = 21; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 91](../tests/contract-playground/src/UninitializedLocalVariables.sol#L91) - - ```solidity - delayedAddressArray[0] = 0x0000000000000000000000000000000000000002; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 92](../tests/contract-playground/src/UninitializedLocalVariables.sol#L92) - - ```solidity - delayedIntArray[0] = -21; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 99](../tests/contract-playground/src/UninitializedLocalVariables.sol#L99) - - ```solidity - address comparison = delayedAddress == 0x0000000000000000000000000000000000000001 ? delayedAddress : address(0); - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 107](../tests/contract-playground/src/UninitializedLocalVariables.sol#L107) - - ```solidity - address arrayComparison = delayedAddressArray[0] == 0x0000000000000000000000000000000000000002 ? delayedAddressArray[0] : address(0); - ``` - - Found in src/WeakRandomness.sol [Line: 25](../tests/contract-playground/src/WeakRandomness.sol#L25) ```solidity @@ -3340,7 +3236,7 @@ Using `ERC721::_mint()` can mint ERC721 tokens to addresses which don't support 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. -
34 Found Instances +
33 Found Instances - Found in src/AdminContract.sol [Line: 2](../tests/contract-playground/src/AdminContract.sol#L2) @@ -3451,12 +3347,6 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai pragma solidity ^0.8.20; ``` -- Found in src/UninitializedLocalVariables.sol [Line: 2](../tests/contract-playground/src/UninitializedLocalVariables.sol#L2) - - ```solidity - pragma solidity ^0.8.0; - ``` - - Found in src/UnsafeERC721Mint.sol [Line: 2](../tests/contract-playground/src/UnsafeERC721Mint.sol#L2) ```solidity @@ -4084,7 +3974,7 @@ Contract contains comments with TODOS 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. -
31 Found Instances +
21 Found Instances - Found in src/Casting.sol [Line: 31](../tests/contract-playground/src/Casting.sol#L31) @@ -4153,66 +4043,6 @@ Consider keeping the naming convention consistent in a given contract. Explicit uint x; ``` -- Found in src/UninitializedLocalVariables.sol [Line: 10](../tests/contract-playground/src/UninitializedLocalVariables.sol#L10) - - ```solidity - uint uninitializedUint; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 16](../tests/contract-playground/src/UninitializedLocalVariables.sol#L16) - - ```solidity - uint[1] memory uninitializedUintArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 25](../tests/contract-playground/src/UninitializedLocalVariables.sol#L25) - - ```solidity - uint initializedUint = 1; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 31](../tests/contract-playground/src/UninitializedLocalVariables.sol#L31) - - ```solidity - uint[1] memory initializedUintArray = [uint(2)]; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 40](../tests/contract-playground/src/UninitializedLocalVariables.sol#L40) - - ```solidity - uint sum = initializedUint + uninitializedUint; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 48](../tests/contract-playground/src/UninitializedLocalVariables.sol#L48) - - ```solidity - uint arraySum = initializedUintArray[0] + uninitializedUintArray[0]; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 63](../tests/contract-playground/src/UninitializedLocalVariables.sol#L63) - - ```solidity - uint delayedUint; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 69](../tests/contract-playground/src/UninitializedLocalVariables.sol#L69) - - ```solidity - uint[1] memory delayedUintArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 97](../tests/contract-playground/src/UninitializedLocalVariables.sol#L97) - - ```solidity - uint sum = delayedUint + 1; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 105](../tests/contract-playground/src/UninitializedLocalVariables.sol#L105) - - ```solidity - uint arraySum = delayedUintArray[0] + 1; - ``` - - Found in src/eth2/DepositContract.sol [Line: 59](../tests/contract-playground/src/eth2/DepositContract.sol#L59) ```solidity @@ -4368,7 +4198,7 @@ Division operations followed directly by multiplication operations can lead to p Remove the redundant statements because no code will be generated and it just congests the codebase. -
30 Found Instances +
6 Found Instances - Found in src/RedundantStatements.sol [Line: 6](../tests/contract-playground/src/RedundantStatements.sol#L6) @@ -4407,30 +4237,6 @@ Remove the redundant statements because no code will be generated and it just co test; // Identifier ``` -- Found in src/UninitializedLocalVariables.sol [Line: 56](../tests/contract-playground/src/UninitializedLocalVariables.sol#L56) - - ```solidity - sum; conjunction; comparison; difference; combinedBytes32; combinedString; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 57](../tests/contract-playground/src/UninitializedLocalVariables.sol#L57) - - ```solidity - arraySum; arrayConjunction; arrayComparison; arrayDifference; arrayCombinedBytes32; arrayCombinedString; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 113](../tests/contract-playground/src/UninitializedLocalVariables.sol#L113) - - ```solidity - sum; conjunction; comparison; difference; combinedBytes32; combinedString; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 114](../tests/contract-playground/src/UninitializedLocalVariables.sol#L114) - - ```solidity - arraySum; arrayConjunction; arrayComparison; arrayDifference; arrayCombinedBytes32; arrayCombinedString; - ``` -
@@ -4474,7 +4280,7 @@ The contract reads it's own variable using `this` which adds an unnecessary STAT State variable appears to be unused. No analysis has been performed to see if any inilne assembly references it. So if that's not the case, consider removing this unused variable. -
26 Found Instances +
25 Found Instances - Found in src/AssemblyExample.sol [Line: 5](../tests/contract-playground/src/AssemblyExample.sol#L5) @@ -4537,12 +4343,6 @@ State variable appears to be unused. No analysis has been performed to see if an uint256 y; ``` -- Found in src/UninitializedLocalVariables.sol [Line: 6](../tests/contract-playground/src/UninitializedLocalVariables.sol#L6) - - ```solidity - uint256 stateVarUint; - ``` - - Found in src/UninitializedStateVariable.sol [Line: 13](../tests/contract-playground/src/UninitializedStateVariable.sol#L13) ```solidity @@ -4701,108 +4501,7 @@ If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. -## L-29: Uninitialized local variables. - -Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability. - -
15 Found Instances - - -- Found in src/ConstantFuncsAssembly.sol [Line: 18](../tests/contract-playground/src/ConstantFuncsAssembly.sol#L18) - - ```solidity - uint256 result; - ``` - -- Found in src/ConstantFuncsAssembly.sol [Line: 27](../tests/contract-playground/src/ConstantFuncsAssembly.sol#L27) - - ```solidity - uint256 result; - ``` - -- Found in src/StorageParameters.sol [Line: 8](../tests/contract-playground/src/StorageParameters.sol#L8) - - ```solidity - uint[1] memory memoryArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 10](../tests/contract-playground/src/UninitializedLocalVariables.sol#L10) - - ```solidity - uint uninitializedUint; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 11](../tests/contract-playground/src/UninitializedLocalVariables.sol#L11) - - ```solidity - bool uninitializedBool; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 12](../tests/contract-playground/src/UninitializedLocalVariables.sol#L12) - - ```solidity - address uninitializedAddress; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 13](../tests/contract-playground/src/UninitializedLocalVariables.sol#L13) - - ```solidity - int uninitializedInt; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 14](../tests/contract-playground/src/UninitializedLocalVariables.sol#L14) - - ```solidity - bytes32 uninitializedBytes32; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 15](../tests/contract-playground/src/UninitializedLocalVariables.sol#L15) - - ```solidity - string memory uninitializedString; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 16](../tests/contract-playground/src/UninitializedLocalVariables.sol#L16) - - ```solidity - uint[1] memory uninitializedUintArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 17](../tests/contract-playground/src/UninitializedLocalVariables.sol#L17) - - ```solidity - bool[1] memory uninitializedBoolArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 18](../tests/contract-playground/src/UninitializedLocalVariables.sol#L18) - - ```solidity - address[1] memory uninitializedAddressArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 19](../tests/contract-playground/src/UninitializedLocalVariables.sol#L19) - - ```solidity - int[1] memory uninitializedIntArray; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 20](../tests/contract-playground/src/UninitializedLocalVariables.sol#L20) - - ```solidity - bytes32[1] memory uninitializedBytes32Array; - ``` - -- Found in src/UninitializedLocalVariables.sol [Line: 21](../tests/contract-playground/src/UninitializedLocalVariables.sol#L21) - - ```solidity - string[1] memory uninitializedStringArray; - ``` - -
- - - -## L-30: Return Bomb +## L-29: Return Bomb A low level callee may consume all callers gas unexpectedly. Avoid unlimited implicit decoding of returndata on calls to unchecked addresses. You can limit the gas by passing a gas limit as an option to the call. For example, `unknownAdress.call{gas: gasLimitHere}("calldata")` That would act as a safety net from OOG errors. diff --git a/reports/report.sarif b/reports/report.sarif index c1d218a6c..8138e38af 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -59,50 +59,6 @@ "byteOffset": 887 } } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 1994 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 2692 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 4694 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 5230 - } - } } ], "message": { @@ -1891,17 +1847,6 @@ } } }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 12, - "byteOffset": 95 - } - } - }, { "physicalLocation": { "artifactLocation": { @@ -2243,28 +2188,6 @@ "byteOffset": 1691 } } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 20, - "byteOffset": 4375 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 27, - "byteOffset": 4853 - } - } } ], "message": { @@ -3621,17 +3544,6 @@ } } }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 23, - "byteOffset": 33 - } - } - }, { "physicalLocation": { "artifactLocation": { @@ -4013,28 +3925,6 @@ } } }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 124 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 17, - "byteOffset": 3093 - } - } - }, { "physicalLocation": { "artifactLocation": { @@ -4627,83 +4517,6 @@ } } }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 42, - "byteOffset": 1204 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 42, - "byteOffset": 3846 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 2, - "byteOffset": 4027 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 42, - "byteOffset": 4101 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 2, - "byteOffset": 4175 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 42, - "byteOffset": 4444 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 42, - "byteOffset": 4942 - } - } - }, { "physicalLocation": { "artifactLocation": { @@ -5442,17 +5255,6 @@ } } }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 23, - "byteOffset": 33 - } - } - }, { "physicalLocation": { "artifactLocation": { @@ -6665,289 +6467,179 @@ { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 17, - "byteOffset": 211 + "byteLength": 27, + "byteOffset": 4611 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 22, - "byteOffset": 434 + "byteLength": 17, + "byteOffset": 4732 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 15, - "byteOffset": 770 + "byteLength": 6, + "byteOffset": 5020 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 20, - "byteOffset": 1062 + "byteLength": 4, + "byteOffset": 5307 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 3, - "byteOffset": 1538 + "byteLength": 6, + "byteOffset": 5347 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 8, - "byteOffset": 2110 + "byteLength": 14, + "byteOffset": 6636 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 11, - "byteOffset": 3205 + "byteLength": 4, + "byteOffset": 8101 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/eth2/DepositContract.sol" }, "region": { - "byteLength": 16, - "byteOffset": 3392 + "byteLength": 6, + "byteOffset": 8141 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/nested_mappings/LaterVersion.sol" }, "region": { - "byteLength": 3, - "byteOffset": 4325 + "byteLength": 5, + "byteOffset": 184 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" + "uri": "src/nested_mappings/NestedMappings.sol" }, "region": { - "byteLength": 8, - "byteOffset": 4785 + "byteLength": 10, + "byteOffset": 168 } } - }, + } + ], + "message": { + "text": "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." + }, + "ruleId": "inconsistent-type-names" + }, + { + "level": "note", + "locations": [ { "physicalLocation": { "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" + "uri": "src/UnusedError.sol" }, "region": { "byteLength": 27, - "byteOffset": 4611 + "byteOffset": 84 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" + "uri": "src/UnusedError.sol" }, "region": { - "byteLength": 17, - "byteOffset": 4732 + "byteLength": 36, + "byteOffset": 258 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" + "uri": "src/WrongOrderOfLayout.sol" }, "region": { - "byteLength": 6, - "byteOffset": 5020 + "byteLength": 21, + "byteOffset": 274 } } - }, + } + ], + "message": { + "text": "it is recommended that the definition be removed when custom error is unused" + }, + "ruleId": "useless-error" + }, + { + "level": "note", + "locations": [ { "physicalLocation": { "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" + "uri": "src/RevertsAndRequriesInLoops.sol" }, "region": { - "byteLength": 4, - "byteOffset": 5307 + "byteLength": 129, + "byteOffset": 227 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" - }, - "region": { - "byteLength": 6, - "byteOffset": 5347 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" - }, - "region": { - "byteLength": 14, - "byteOffset": 6636 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" - }, - "region": { - "byteLength": 4, - "byteOffset": 8101 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" - }, - "region": { - "byteLength": 6, - "byteOffset": 8141 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/nested_mappings/LaterVersion.sol" - }, - "region": { - "byteLength": 5, - "byteOffset": 184 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/nested_mappings/NestedMappings.sol" - }, - "region": { - "byteLength": 10, - "byteOffset": 168 - } - } - } - ], - "message": { - "text": "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." - }, - "ruleId": "inconsistent-type-names" - }, - { - "level": "note", - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UnusedError.sol" - }, - "region": { - "byteLength": 27, - "byteOffset": 84 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UnusedError.sol" - }, - "region": { - "byteLength": 36, - "byteOffset": 258 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/WrongOrderOfLayout.sol" - }, - "region": { - "byteLength": 21, - "byteOffset": 274 - } - } - } - ], - "message": { - "text": "it is recommended that the definition be removed when custom error is unused" - }, - "ruleId": "useless-error" - }, - { - "level": "note", - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/RevertsAndRequriesInLoops.sol" - }, - "region": { - "byteLength": 129, - "byteOffset": 227 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/RevertsAndRequriesInLoops.sol" + "uri": "src/RevertsAndRequriesInLoops.sol" }, "region": { "byteLength": 150, @@ -7082,270 +6774,6 @@ "byteOffset": 377 } } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 3, - "byteOffset": 2871 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 11, - "byteOffset": 2876 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 10, - "byteOffset": 2889 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 10, - "byteOffset": 2901 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 2913 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 14, - "byteOffset": 2930 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 8, - "byteOffset": 2958 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 2968 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 2986 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 3003 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 20, - "byteOffset": 3020 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 19, - "byteOffset": 3042 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 3, - "byteOffset": 5369 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 11, - "byteOffset": 5374 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 10, - "byteOffset": 5387 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 10, - "byteOffset": 5399 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 5411 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 14, - "byteOffset": 5428 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 8, - "byteOffset": 5452 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 5462 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 5480 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 5497 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 20, - "byteOffset": 5514 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 19, - "byteOffset": 5536 - } - } } ], "message": { @@ -7519,17 +6947,6 @@ } } }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 12, - "byteOffset": 95 - } - } - }, { "physicalLocation": { "artifactLocation": { @@ -7796,180 +7213,6 @@ }, "ruleId": "boolean-equality" }, - { - "level": "note", - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/ConstantFuncsAssembly.sol" - }, - "region": { - "byteLength": 14, - "byteOffset": 478 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/ConstantFuncsAssembly.sol" - }, - "region": { - "byteLength": 14, - "byteOffset": 716 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/StorageParameters.sol" - }, - "region": { - "byteLength": 11, - "byteOffset": 184 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 17, - "byteOffset": 211 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 17, - "byteOffset": 243 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 20, - "byteOffset": 278 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 16, - "byteOffset": 312 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 20, - "byteOffset": 346 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 19, - "byteOffset": 390 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 22, - "byteOffset": 434 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 22, - "byteOffset": 481 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 25, - "byteOffset": 531 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 21, - "byteOffset": 580 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 25, - "byteOffset": 629 - } - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "src/UninitializedLocalVariables.sol" - }, - "region": { - "byteLength": 24, - "byteOffset": 681 - } - } - } - ], - "message": { - "text": "Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability." - }, - "ruleId": "uninitialized-local-variable" - }, { "level": "note", "locations": [ diff --git a/reports/templegold-report.md b/reports/templegold-report.md index f8359c805..65d230eae 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -40,7 +40,6 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-19: Redundant statements have no effect.](#l-19-redundant-statements-have-no-effect) - [L-20: Potentially unused `private` / `internal` state variables found.](#l-20-potentially-unused-private--internal-state-variables-found) - [L-21: Boolean equality is not required.](#l-21-boolean-equality-is-not-required) - - [L-22: Uninitialized local variables.](#l-22-uninitialized-local-variables) # Summary @@ -194,7 +193,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 10 | -| Low | 22 | +| Low | 21 | # High Issues @@ -8702,92 +8701,3 @@ If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. -## L-22: Uninitialized local variables. - -Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability. - -
13 Found Instances - - -- Found in contracts/admin/TempleTeamPaymentsFactory.sol [Line: 128](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPaymentsFactory.sol#L128) - - ```solidity - for (uint256 i; i < _dests.length; ) { - ``` - -- Found in contracts/admin/TempleTeamPaymentsV2.sol [Line: 50](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPaymentsV2.sol#L50) - - ```solidity - for (uint256 i; i < _addresses.length; ) { - ``` - -- Found in contracts/amo/AuraStaking.sol [Line: 116](../tests/2024-07-templegold/protocol/contracts/amo/AuraStaking.sol#L116) - - ```solidity - for (uint i; i < length; ++i) { - ``` - -- Found in contracts/fakes/UniswapV2Router02NoEth.sol [Line: 178](../tests/2024-07-templegold/protocol/contracts/fakes/UniswapV2Router02NoEth.sol#L178) - - ```solidity - for (uint i; i < path.length - 1; i++) { - ``` - -- Found in contracts/fakes/UniswapV2Router02NoEth.sol [Line: 259](../tests/2024-07-templegold/protocol/contracts/fakes/UniswapV2Router02NoEth.sol#L259) - - ```solidity - for (uint i; i < path.length - 1; i++) { - ``` - -- Found in contracts/v2/TempleDebtToken.sol [Line: 454](../tests/2024-07-templegold/protocol/contracts/v2/TempleDebtToken.sol#L454) - - ```solidity - for (uint256 i; i < _length; ++i) { - ``` - -- Found in contracts/v2/TreasuryReservesVault.sol [Line: 187](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L187) - - ```solidity - for (uint256 i; i < _length; ++i) { - ``` - -- Found in contracts/v2/TreasuryReservesVault.sol [Line: 294](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L294) - - ```solidity - for (uint256 i; i < _length; ++i) { - ``` - -- Found in contracts/v2/access/TempleElevatedAccess.sol [Line: 112](../tests/2024-07-templegold/protocol/contracts/v2/access/TempleElevatedAccess.sol#L112) - - ```solidity - for (uint256 i; i < _length; ++i) { - ``` - -- Found in contracts/v2/circuitBreaker/TempleCircuitBreakerAllUsersPerPeriod.sol [Line: 174](../tests/2024-07-templegold/protocol/contracts/v2/circuitBreaker/TempleCircuitBreakerAllUsersPerPeriod.sol#L174) - - ```solidity - for (uint256 i; i < _nBuckets; ++i) { - ``` - -- Found in contracts/v2/safeGuards/ThresholdSafeGuard.sol [Line: 126](../tests/2024-07-templegold/protocol/contracts/v2/safeGuards/ThresholdSafeGuard.sol#L126) - - ```solidity - for (uint256 i; i < length; ++i) { - ``` - -- Found in contracts/v2/strategies/AbstractStrategy.sol [Line: 100](../tests/2024-07-templegold/protocol/contracts/v2/strategies/AbstractStrategy.sol#L100) - - ```solidity - for (uint256 i; i < _length; ++i) { - ``` - -- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 330](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L330) - - ```solidity - for (uint256 i; i < _numAccounts; ++i) { - ``` - -
- - - diff --git a/tests/contract-playground/src/UninitializedLocalVariables.sol b/tests/contract-playground/src/UninitializedLocalVariables.sol deleted file mode 100644 index 4bfd9a707..000000000 --- a/tests/contract-playground/src/UninitializedLocalVariables.sol +++ /dev/null @@ -1,117 +0,0 @@ -// SPDX-License-Identifier: MITc -pragma solidity ^0.8.0; - -contract AllDataTypes { - - uint256 stateVarUint; - - function testAllDataTypes() public pure { - // Uninitialized variables (BAD) - uint uninitializedUint; - bool uninitializedBool; - address uninitializedAddress; - int uninitializedInt; - bytes32 uninitializedBytes32; - string memory uninitializedString; - uint[1] memory uninitializedUintArray; - bool[1] memory uninitializedBoolArray; - address[1] memory uninitializedAddressArray; - int[1] memory uninitializedIntArray; - bytes32[1] memory uninitializedBytes32Array; - string[1] memory uninitializedStringArray; - - - // Initialized variables (GOOD) - uint initializedUint = 1; - bool initializedBool = true; - address initializedAddress = 0x0000000000000000000000000000000000000000; - int initializedInt = -1; - bytes32 initializedBytes32 = "hello"; - string memory initializedString = "world"; - uint[1] memory initializedUintArray = [uint(2)]; - bool[1] memory initializedBoolArray = [false]; - address[1] memory initializedAddressArray = [0x0000000000000000000000000000000000000001]; - int[1] memory initializedIntArray = [int(-2)]; - bytes32[1] memory initializedBytes32Array = [bytes32("bye")]; - string[1] memory initializedStringArray = ["Solidity"]; - - { - // Example usage of initialized and uninitialized variables - uint sum = initializedUint + uninitializedUint; - bool conjunction = initializedBool && uninitializedBool; - address comparison = initializedAddress == uninitializedAddress ? initializedAddress : uninitializedAddress; - int difference = initializedInt - uninitializedInt; - bytes32 combinedBytes32 = keccak256(abi.encodePacked(initializedBytes32, uninitializedBytes32)); - string memory combinedString = string(abi.encodePacked(initializedString, uninitializedString)); - - // Example usage of arrays - uint arraySum = initializedUintArray[0] + uninitializedUintArray[0]; - bool arrayConjunction = initializedBoolArray[0] && uninitializedBoolArray[0]; - address arrayComparison = initializedAddressArray[0] == uninitializedAddressArray[0] ? initializedAddressArray[0] : uninitializedAddressArray[0]; - int arrayDifference = initializedIntArray[0] - uninitializedIntArray[0]; - bytes32 arrayCombinedBytes32 = keccak256(abi.encodePacked(initializedBytes32Array[0], uninitializedBytes32Array[0])); - string memory arrayCombinedString = string(abi.encodePacked(initializedStringArray[0], uninitializedStringArray[0])); - - // These statements are to prevent warnings about unused variables - sum; conjunction; comparison; difference; combinedBytes32; combinedString; - arraySum; arrayConjunction; arrayComparison; arrayDifference; arrayCombinedBytes32; arrayCombinedString; - } - } - - function testAllDataTypes2() public pure { - // Declaration of variables (but initialized later) GOOD - uint delayedUint; - bool delayedBool; - address delayedAddress; - int delayedInt; - bytes32 delayedBytes32; - string memory delayedString; - uint[1] memory delayedUintArray; - bool[1] memory delayedBoolArray; - address[1] memory delayedAddressArray; - int[1] memory delayedIntArray; - bytes32[1] memory delayedBytes32Array; - string[1] memory delayedStringArray; - - - // Initialize delayedUint via assembly - assembly ("memory-safe") { - delayedUint := 6 - } - - - // Initialization of variables - delayedBool = true; - delayedAddress = 0x0000000000000000000000000000000000000001; - delayedInt = -42; - delayedBytes32 = "example"; - delayedString = "initialized later"; - delayedUintArray[0] = 21; - delayedBoolArray[0] = false; - delayedAddressArray[0] = 0x0000000000000000000000000000000000000002; - delayedIntArray[0] = -21; - delayedBytes32Array[0] = "test"; - delayedStringArray[0] = "array"; - - // Example usage of initialized variables - uint sum = delayedUint + 1; - bool conjunction = delayedBool && false; - address comparison = delayedAddress == 0x0000000000000000000000000000000000000001 ? delayedAddress : address(0); - int difference = delayedInt - 1; - bytes32 combinedBytes32 = keccak256(abi.encodePacked(delayedBytes32, "concat")); - string memory combinedString = string(abi.encodePacked(delayedString, " now")); - - // Example usage of arrays - uint arraySum = delayedUintArray[0] + 1; - bool arrayConjunction = delayedBoolArray[0] && true; - address arrayComparison = delayedAddressArray[0] == 0x0000000000000000000000000000000000000002 ? delayedAddressArray[0] : address(0); - int arrayDifference = delayedIntArray[0] - 1; - bytes32 arrayCombinedBytes32 = keccak256(abi.encodePacked(delayedBytes32Array[0], " more")); - string memory arrayCombinedString = string(abi.encodePacked(delayedStringArray[0], " elements")); - - // These statements are to prevent warnings about unused variables - sum; conjunction; comparison; difference; combinedBytes32; combinedString; - arraySum; arrayConjunction; arrayComparison; arrayDifference; arrayCombinedBytes32; arrayCombinedString; - } - -}