From 1869b11826a0ad216a32fc594e8661fdb46a764e Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 24 Aug 2024 12:11:51 +0530 Subject: [PATCH 01/12] detector --- aderyn_core/src/detect/detector.rs | 5 + .../src/detect/low/missing_inheritance.rs | 168 ++++++++++++++++++ aderyn_core/src/detect/low/mod.rs | 2 + .../src/MissingInheritance.sol | 26 +++ 4 files changed, 201 insertions(+) create mode 100644 aderyn_core/src/detect/low/missing_inheritance.rs create mode 100644 tests/contract-playground/src/MissingInheritance.sol diff --git a/aderyn_core/src/detect/detector.rs b/aderyn_core/src/detect/detector.rs index f534193a..bf77c9f3 100644 --- a/aderyn_core/src/detect/detector.rs +++ b/aderyn_core/src/detect/detector.rs @@ -93,6 +93,7 @@ pub fn get_all_issue_detectors() -> Vec> { Box::::default(), Box::::default(), Box::::default(), + Box::::default(), ] } @@ -104,6 +105,7 @@ pub fn get_all_detectors_names() -> Vec { #[derive(Debug, PartialEq, EnumString, Display)] #[strum(serialize_all = "kebab-case")] pub(crate) enum IssueDetectorNamePool { + MissingInheritance, FunctionSelectorCollision, CacheArrayLength, AssertStateChange, @@ -189,6 +191,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option { + Some(Box::::default()) + } IssueDetectorNamePool::FunctionSelectorCollision => { Some(Box::::default()) } diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs new file mode 100644 index 00000000..09992116 --- /dev/null +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -0,0 +1,168 @@ +use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::error::Error; + +use crate::ast::{ASTNode, ContractKind, NodeID}; + +use crate::capture; +use crate::context::browser::ExtractVariableDeclarations; +use crate::detect::detector::IssueDetectorNamePool; +use crate::{ + context::workspace_context::WorkspaceContext, + detect::detector::{IssueDetector, IssueSeverity}, +}; +use eyre::Result; + +#[derive(Default)] +pub struct MissingInheritanceDetector { + // 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 MissingInheritanceDetector { + fn detect(&mut self, context: &WorkspaceContext) -> Result> { + // Key -> Interface ID, Value -> Collection of function selectors in the inheritance + let mut interface_function_selectors: HashMap> = Default::default(); + + for interface in context + .contract_definitions() + .into_iter() + .filter(|&c| c.kind == ContractKind::Interface) + { + if let Some(full_interface) = &interface.linearized_base_contracts { + for interface_node_id in full_interface { + if let Some(ASTNode::ContractDefinition(interface_node)) = + context.nodes.get(interface_node_id) + { + let selectors: Vec = interface_node + .function_definitions() + .iter() + .flat_map(|f| f.function_selector.clone()) + .collect(); + interface_function_selectors + .entry(interface.id) + .or_insert(selectors); + } + } + } + } + + // Key -> Contract ID, Value -> Collection of function selectors in the contract + let mut contract_function_selectors: HashMap> = Default::default(); + + // Key -> Contract ID, Value -> Set of contract/interface IDs in it's heirarchy + let mut inheritance_map: HashMap> = Default::default(); + + for contract in context + .contract_definitions() + .into_iter() + .filter(|&c| c.kind == ContractKind::Contract) + { + if let Some(full_contract) = &contract.linearized_base_contracts { + inheritance_map + .entry(contract.id) + .or_insert(BTreeSet::from_iter(full_contract.iter().copied())); + + for contract_node_id in full_contract { + if let Some(ASTNode::ContractDefinition(contract_node)) = + context.nodes.get(contract_node_id) + { + let function_selectors: Vec = contract_node + .function_definitions() + .iter() + .flat_map(|f| f.function_selector.clone()) + .collect(); + + let all_variables = + ExtractVariableDeclarations::from(contract_node).extracted; + + let state_variable_function_selectors: Vec = all_variables + .into_iter() + .flat_map(|v| v.function_selector.clone()) + .collect(); + + let mut all_function_selectors = Vec::with_capacity( + function_selectors.len() + state_variable_function_selectors.len(), + ); + all_function_selectors.extend(function_selectors); + all_function_selectors.extend(state_variable_function_selectors); + + contract_function_selectors + .entry(contract.id) + .or_insert(all_function_selectors); + } + } + } + } + + for (contract_id, contract_function_selectors) in contract_function_selectors { + let inheritances = inheritance_map.entry(contract_id).or_default(); + for (_potentially_missing_interface, interface_function_selectors) in + interface_function_selectors + .iter() + .filter(|(&k, _)| !inheritances.contains(&k)) + { + if interface_function_selectors + .into_iter() + .all(|s| contract_function_selectors.contains(s)) + { + // Now we know that `_potentially_missing_interface` is missing inheritance for `contract_id` + if let Some(contract) = context.nodes.get(&contract_id) { + capture!(self, context, contract); + } + } + } + } + + Ok(!self.found_instances.is_empty()) + } + + fn severity(&self) -> IssueSeverity { + IssueSeverity::Low + } + + fn title(&self) -> String { + String::from("Potentially missing inheritance for contract.") + } + + fn description(&self) -> String { + String::from("There is an interface that is potentially missing (not included in) the inheritance of this contract.") + } + + fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { + self.found_instances.clone() + } + + fn name(&self) -> String { + format!("{}", IssueDetectorNamePool::MissingInheritance) + } +} + +#[cfg(test)] +mod missing_inheritance_tests { + use serial_test::serial; + + use crate::detect::{ + detector::IssueDetector, low::missing_inheritance::MissingInheritanceDetector, + }; + + #[test] + #[serial] + fn test_missing_inheritance() { + let context = crate::detect::test_utils::load_solidity_source_unit( + "../tests/contract-playground/src/MissingInheritance.sol", + ); + + let mut detector = MissingInheritanceDetector::default(); + let found = detector.detect(&context).unwrap(); + // assert that the detector found an issue + assert!(found); + // assert that the detector found the correct number of instances + assert_eq!(detector.instances().len(), 1); + // assert the severity is low + assert_eq!( + detector.severity(), + crate::detect::detector::IssueSeverity::Low + ); + } +} diff --git a/aderyn_core/src/detect/low/mod.rs b/aderyn_core/src/detect/low/mod.rs index 19e54c4c..438949e8 100644 --- a/aderyn_core/src/detect/low/mod.rs +++ b/aderyn_core/src/detect/low/mod.rs @@ -14,6 +14,7 @@ pub(crate) mod empty_blocks; pub(crate) mod function_init_state_vars; pub(crate) mod inconsistent_type_names; pub(crate) mod large_literal_value; +pub(crate) mod missing_inheritance; pub(crate) mod non_reentrant_before_others; pub(crate) mod public_variable_read_in_external_context; pub(crate) mod push_0_opcode; @@ -49,6 +50,7 @@ pub use empty_blocks::EmptyBlockDetector; pub use function_init_state_vars::FunctionInitializingStateDetector; pub use inconsistent_type_names::InconsistentTypeNamesDetector; pub use large_literal_value::LargeLiteralValueDetector; +pub use missing_inheritance::MissingInheritanceDetector; pub use non_reentrant_before_others::NonReentrantBeforeOthersDetector; pub use public_variable_read_in_external_context::PublicVariableReadInExternalContextDetector; pub use push_0_opcode::PushZeroOpcodeDetector; diff --git a/tests/contract-playground/src/MissingInheritance.sol b/tests/contract-playground/src/MissingInheritance.sol new file mode 100644 index 00000000..b7f91772 --- /dev/null +++ b/tests/contract-playground/src/MissingInheritance.sol @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +// BAD +contract MissingInheritanceCounter { + uint256 public count; + + function increment() external { + count += 1; + } +} + +interface IMissingInheritanceCounter { + function count() external view returns (uint256); + + function increment() external; +} + +// GOOD +contract MissingInheritanceCounter2 is IMissingInheritanceCounter { + uint256 public count; + + function increment() external { + count += 1; + } +} \ No newline at end of file From 43ae04914f5b78db4e37dbb5faadd779ec918e30 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 24 Aug 2024 12:28:51 +0530 Subject: [PATCH 02/12] cli/reportgen --- .../src/detect/low/missing_inheritance.rs | 10 +- reports/ccip-functions-report.md | 104 +++++++++++++++++- reports/report.json | 56 +++++++++- reports/report.md | 61 +++++++++- reports/report.sarif | 75 +++++++++++++ reports/templegold-report.md | 50 ++++++++- 6 files changed, 342 insertions(+), 14 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index 09992116..4f593470 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -96,14 +96,20 @@ impl IssueDetector for MissingInheritanceDetector { } for (contract_id, contract_function_selectors) in contract_function_selectors { + if contract_function_selectors.is_empty() { + continue; + } let inheritances = inheritance_map.entry(contract_id).or_default(); for (_potentially_missing_interface, interface_function_selectors) in interface_function_selectors .iter() .filter(|(&k, _)| !inheritances.contains(&k)) { + if interface_function_selectors.is_empty() { + continue; + } if interface_function_selectors - .into_iter() + .iter() .all(|s| contract_function_selectors.contains(s)) { // Now we know that `_potentially_missing_interface` is missing inheritance for `contract_id` @@ -126,7 +132,7 @@ impl IssueDetector for MissingInheritanceDetector { } fn description(&self) -> String { - String::from("There is an interface that is potentially missing (not included in) the inheritance of this contract.") + String::from("There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.") } fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index d4d99f0a..83a413ad 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -27,6 +27,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-13: Loop contains `require`/`revert` statements](#l-13-loop-contains-requirerevert-statements) - [L-14: Loop condition contains `state_variable.length` that could be cached outside.](#l-14-loop-condition-contains-statevariablelength-that-could-be-cached-outside) - [L-15: Costly operations inside loops.](#l-15-costly-operations-inside-loops) + - [L-16: Potentially missing inheritance for contract.](#l-16-potentially-missing-inheritance-for-contract) # Summary @@ -103,7 +104,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 3 | -| Low | 15 | +| Low | 16 | # High Issues @@ -2654,3 +2655,104 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local +## L-16: Potentially missing inheritance for contract. + +There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. + +
15 Found Instances + + +- Found in src/v0.8/functions/dev/v1_X/FunctionsBilling.sol [Line: 17](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsBilling.sol#L17) + + ```solidity + abstract contract FunctionsBilling is Routable, IFunctionsBilling { + ``` + +- Found in src/v0.8/functions/dev/v1_X/FunctionsClient.sol [Line: 11](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsClient.sol#L11) + + ```solidity + abstract contract FunctionsClient is IFunctionsClient { + ``` + +- Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L13) + + ```solidity + contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { + ``` + +- Found in src/v0.8/functions/dev/v1_X/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol#L16) + + ```solidity + contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { + ``` + +- Found in src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol [Line: 15](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol#L15) + + ```solidity + abstract contract FunctionsSubscriptions is IFunctionsSubscriptions, IERC677Receiver { + ``` + +- Found in src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol#L14) + + ```solidity + contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { + ``` + +- Found in src/v0.8/functions/v1_0_0/FunctionsClient.sol [Line: 11](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsClient.sol#L11) + + ```solidity + abstract contract FunctionsClient is IFunctionsClient { + ``` + +- Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L13) + + ```solidity + contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { + ``` + +- Found in src/v0.8/functions/v1_0_0/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsRouter.sol#L16) + + ```solidity + contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { + ``` + +- Found in src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol [Line: 15](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol#L15) + + ```solidity + abstract contract FunctionsSubscriptions is IFunctionsSubscriptions, IERC677Receiver { + ``` + +- Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L13) + + ```solidity + contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { + ``` + +- Found in src/v0.8/functions/v1_3_0/FunctionsBilling.sol [Line: 17](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsBilling.sol#L17) + + ```solidity + abstract contract FunctionsBilling is Routable, IFunctionsBilling { + ``` + +- Found in src/v0.8/functions/v1_3_0/FunctionsClient.sol [Line: 11](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsClient.sol#L11) + + ```solidity + abstract contract FunctionsClient is IFunctionsClient { + ``` + +- Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L13) + + ```solidity + contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { + ``` + +- Found in src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol#L14) + + ```solidity + contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { + ``` + +
+ + + diff --git a/reports/report.json b/reports/report.json index d310caea..80e2ee3a 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 92, - "total_sloc": 3230 + "total_source_units": 93, + "total_sloc": 3247 }, "files_details": { "files_details": [ @@ -165,6 +165,10 @@ "file_path": "src/KeccakContract.sol", "n_sloc": 21 }, + { + "file_path": "src/MissingInheritance.sol", + "n_sloc": 17 + }, { "file_path": "src/MisusedBoolean.sol", "n_sloc": 67 @@ -377,7 +381,7 @@ }, "issue_count": { "high": 41, - "low": 34 + "low": 35 }, "high_issues": { "issues": [ @@ -2686,6 +2690,12 @@ "src": "32:23", "src_char": "32:23" }, + { + "contract_path": "src/MissingInheritance.sol", + "line_no": 2, + "src": "32:24", + "src_char": "32:24" + }, { "contract_path": "src/MsgValueInLoop.sol", "line_no": 2, @@ -3930,6 +3940,12 @@ "src": "32:23", "src_char": "32:23" }, + { + "contract_path": "src/MissingInheritance.sol", + "line_no": 2, + "src": "32:24", + "src_char": "32:24" + }, { "contract_path": "src/MsgValueInLoop.sol", "line_no": 2, @@ -5331,6 +5347,37 @@ "src_char": "414:15" } ] + }, + { + "title": "Potentially missing inheritance for contract.", + "description": "There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.", + "detector_name": "missing-inheritance", + "instances": [ + { + "contract_path": "src/MissingInheritance.sol", + "line_no": 5, + "src": "74:25", + "src_char": "74:25" + }, + { + "contract_path": "src/TestERC20.sol", + "line_no": 4, + "src": "70:9", + "src_char": "70:9" + }, + { + "contract_path": "src/inheritance/ExtendedInheritance.sol", + "line_no": 6, + "src": "99:19", + "src_char": "99:19" + }, + { + "contract_path": "src/inheritance/InheritanceBase.sol", + "line_no": 6, + "src": "104:15", + "src_char": "104:15" + } + ] } ] }, @@ -5409,6 +5456,7 @@ "costly-operations-inside-loops", "constant-function-changing-state", "builtin-symbol-shadow", - "function-selector-collision" + "function-selector-collision", + "missing-inheritance" ] } \ No newline at end of file diff --git a/reports/report.md b/reports/report.md index 3c3320ae..7ce9d7fb 100644 --- a/reports/report.md +++ b/reports/report.md @@ -84,6 +84,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-32: Incorrect use of `assert()`](#l-32-incorrect-use-of-assert) - [L-33: Costly operations inside loops.](#l-33-costly-operations-inside-loops) - [L-34: Builtin Symbol Shadowing](#l-34-builtin-symbol-shadowing) + - [L-35: Potentially missing inheritance for contract.](#l-35-potentially-missing-inheritance-for-contract) # Summary @@ -92,8 +93,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 92 | -| Total nSLOC | 3230 | +| .sol Files | 93 | +| Total nSLOC | 3247 | ## Files Details @@ -140,6 +141,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/IncorrectShift.sol | 17 | | src/InternalFunctions.sol | 22 | | src/KeccakContract.sol | 21 | +| src/MissingInheritance.sol | 17 | | src/MisusedBoolean.sol | 67 | | src/MsgValueInLoop.sol | 55 | | src/MultipleConstructorSchemes.sol | 10 | @@ -192,7 +194,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** | **3230** | +| **Total** | **3247** | ## Issue Summary @@ -200,7 +202,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 41 | -| Low | 34 | +| Low | 35 | # High Issues @@ -2590,7 +2592,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;` -
34 Found Instances +
35 Found Instances - Found in src/BuiltinSymbolShadow.sol [Line: 2](../tests/contract-playground/src/BuiltinSymbolShadow.sol#L2) @@ -2701,6 +2703,12 @@ Consider using a specific version of Solidity in your contracts instead of a wid pragma solidity ^0.8.0; ``` +- Found in src/MissingInheritance.sol [Line: 2](../tests/contract-playground/src/MissingInheritance.sol#L2) + + ```solidity + pragma solidity ^0.8.24; + ``` + - Found in src/MsgValueInLoop.sol [Line: 2](../tests/contract-playground/src/MsgValueInLoop.sol#L2) ```solidity @@ -3854,7 +3862,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. -
40 Found Instances +
41 Found Instances - Found in src/AdminContract.sol [Line: 2](../tests/contract-playground/src/AdminContract.sol#L2) @@ -3953,6 +3961,12 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai pragma solidity 0.8.20; ``` +- Found in src/MissingInheritance.sol [Line: 2](../tests/contract-playground/src/MissingInheritance.sol#L2) + + ```solidity + pragma solidity ^0.8.24; + ``` + - Found in src/MsgValueInLoop.sol [Line: 2](../tests/contract-playground/src/MsgValueInLoop.sol#L2) ```solidity @@ -5438,3 +5452,38 @@ Name clashes with a built-in-symbol. Consider renaming it. +## L-35: Potentially missing inheritance for contract. + +There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. + +
4 Found Instances + + +- Found in src/MissingInheritance.sol [Line: 5](../tests/contract-playground/src/MissingInheritance.sol#L5) + + ```solidity + contract MissingInheritanceCounter { + ``` + +- Found in src/TestERC20.sol [Line: 4](../tests/contract-playground/src/TestERC20.sol#L4) + + ```solidity + contract TestERC20 { + ``` + +- Found in src/inheritance/ExtendedInheritance.sol [Line: 6](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L6) + + ```solidity + contract ExtendedInheritance is InheritanceBase { + ``` + +- Found in src/inheritance/InheritanceBase.sol [Line: 6](../tests/contract-playground/src/inheritance/InheritanceBase.sol#L6) + + ```solidity + contract InheritanceBase is IContractInheritance { + ``` + +
+ + + diff --git a/reports/report.sarif b/reports/report.sarif index 0c243290..4ac46179 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -4040,6 +4040,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MissingInheritance.sol" + }, + "region": { + "byteLength": 24, + "byteOffset": 32 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -6290,6 +6301,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MissingInheritance.sol" + }, + "region": { + "byteLength": 24, + "byteOffset": 32 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -8785,6 +8807,59 @@ "text": "Name clashes with a built-in-symbol. Consider renaming it." }, "ruleId": "builtin-symbol-shadow" + }, + { + "level": "note", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MissingInheritance.sol" + }, + "region": { + "byteLength": 25, + "byteOffset": 74 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/TestERC20.sol" + }, + "region": { + "byteLength": 9, + "byteOffset": 70 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/inheritance/ExtendedInheritance.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 99 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/inheritance/InheritanceBase.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 104 + } + } + } + ], + "message": { + "text": "There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces." + }, + "ruleId": "missing-inheritance" } ], "tool": { diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 29cd97e8..35e2e340 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -41,6 +41,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [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: Costly operations inside loops.](#l-22-costly-operations-inside-loops) + - [L-23: Potentially missing inheritance for contract.](#l-23-potentially-missing-inheritance-for-contract) # Summary @@ -194,7 +195,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 10 | -| Low | 22 | +| Low | 23 | # High Issues @@ -8821,3 +8822,50 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local +## L-23: Potentially missing inheritance for contract. + +There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. + +
6 Found Instances + + +- Found in contracts/amm/TreasuryIV.sol [Line: 9](../tests/2024-07-templegold/protocol/contracts/amm/TreasuryIV.sol#L9) + + ```solidity + contract TreasuryIV is Ownable { + ``` + +- Found in contracts/core/RebasingERC20.sol [Line: 13](../tests/2024-07-templegold/protocol/contracts/core/RebasingERC20.sol#L13) + + ```solidity + abstract contract RebasingERC20 is ERC20 { + ``` + +- Found in contracts/fakes/FakeERC20.sol [Line: 10](../tests/2024-07-templegold/protocol/contracts/fakes/FakeERC20.sol#L10) + + ```solidity + contract FakeERC20 is ERC20 { + ``` + +- Found in contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol [Line: 7](../tests/2024-07-templegold/protocol/contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol#L7) + + ```solidity + contract TempleDebtTokenTestnetAdmin { + ``` + +- Found in contracts/templegold/TempleGoldAdmin.sol [Line: 20](../tests/2024-07-templegold/protocol/contracts/templegold/TempleGoldAdmin.sol#L20) + + ```solidity + contract TempleGoldAdmin is ITempleGoldAdmin, TempleElevatedAccess { + ``` + +- Found in contracts/v2/TempleDebtToken.sol [Line: 39](../tests/2024-07-templegold/protocol/contracts/v2/TempleDebtToken.sol#L39) + + ```solidity + contract TempleDebtToken is ITempleDebtToken, TempleElevatedAccess { + ``` + +
+ + + From 2996432a0f28d50dcb0cb5c70b367515eb774a09 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 24 Aug 2024 12:58:38 +0530 Subject: [PATCH 03/12] fix + reportgen --- .../src/detect/low/missing_inheritance.rs | 88 +++++++++---------- reports/ccip-functions-report.md | 46 +--------- reports/report.json | 2 +- reports/report.md | 2 +- reports/report.sarif | 2 +- reports/templegold-report.md | 10 +-- 6 files changed, 49 insertions(+), 101 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index 4f593470..3522e34b 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -1,4 +1,5 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::convert::identity; use std::error::Error; use crate::ast::{ASTNode, ContractKind, NodeID}; @@ -21,43 +22,13 @@ pub struct MissingInheritanceDetector { impl IssueDetector for MissingInheritanceDetector { fn detect(&mut self, context: &WorkspaceContext) -> Result> { - // Key -> Interface ID, Value -> Collection of function selectors in the inheritance - let mut interface_function_selectors: HashMap> = Default::default(); - - for interface in context - .contract_definitions() - .into_iter() - .filter(|&c| c.kind == ContractKind::Interface) - { - if let Some(full_interface) = &interface.linearized_base_contracts { - for interface_node_id in full_interface { - if let Some(ASTNode::ContractDefinition(interface_node)) = - context.nodes.get(interface_node_id) - { - let selectors: Vec = interface_node - .function_definitions() - .iter() - .flat_map(|f| f.function_selector.clone()) - .collect(); - interface_function_selectors - .entry(interface.id) - .or_insert(selectors); - } - } - } - } - // Key -> Contract ID, Value -> Collection of function selectors in the contract let mut contract_function_selectors: HashMap> = Default::default(); // Key -> Contract ID, Value -> Set of contract/interface IDs in it's heirarchy let mut inheritance_map: HashMap> = Default::default(); - for contract in context - .contract_definitions() - .into_iter() - .filter(|&c| c.kind == ContractKind::Contract) - { + for contract in context.contract_definitions() { if let Some(full_contract) = &contract.linearized_base_contracts { inheritance_map .entry(contract.id) @@ -95,26 +66,48 @@ impl IssueDetector for MissingInheritanceDetector { } } - for (contract_id, contract_function_selectors) in contract_function_selectors { - if contract_function_selectors.is_empty() { + for (contract_id, contract_selectors) in &contract_function_selectors { + if contract_selectors.is_empty() { continue; } - let inheritances = inheritance_map.entry(contract_id).or_default(); - for (_potentially_missing_interface, interface_function_selectors) in - interface_function_selectors - .iter() - .filter(|(&k, _)| !inheritances.contains(&k)) + if let Some(ASTNode::ContractDefinition(c)) = context.nodes.get(contract_id) { + if c.kind != ContractKind::Contract || c.is_abstract.map_or(false, identity) { + continue; + } + } + let inheritances = inheritance_map.entry(*contract_id).or_default(); + for (potentially_missing_inheritance, missing_function_selectors) in + &contract_function_selectors { - if interface_function_selectors.is_empty() { + // Check that it's not empty + if missing_function_selectors.is_empty() { + continue; + } + + // Check that it's not the same contract + if potentially_missing_inheritance == contract_id { continue; } - if interface_function_selectors - .iter() - .all(|s| contract_function_selectors.contains(s)) + + // Check that it's not already inherited + if inheritances.contains(potentially_missing_inheritance) { + continue; + } + + if let Some(ASTNode::ContractDefinition(c)) = + context.nodes.get(potentially_missing_inheritance) { - // Now we know that `_potentially_missing_interface` is missing inheritance for `contract_id` - if let Some(contract) = context.nodes.get(&contract_id) { - capture!(self, context, contract); + if c.kind == ContractKind::Interface || c.is_abstract.map_or(false, identity) { + // Check that the contract is compatible with the missing inheritance + if missing_function_selectors + .iter() + .all(|s| contract_selectors.contains(s)) + { + // Now we know that `_potentially_missing_inheritance` is missing inheritance for `contract_id` + if let Some(contract) = context.nodes.get(&contract_id) { + capture!(self, context, contract); + } + } } } } @@ -132,7 +125,7 @@ impl IssueDetector for MissingInheritanceDetector { } fn description(&self) -> String { - String::from("There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.") + String::from("There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.") } fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { @@ -163,6 +156,9 @@ mod missing_inheritance_tests { let found = detector.detect(&context).unwrap(); // assert that the detector found an issue assert!(found); + + println!("{:#?}", detector.instances()); + // assert that the detector found the correct number of instances assert_eq!(detector.instances().len(), 1); // assert the severity is low diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 83a413ad..5e32b0fd 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -2657,23 +2657,11 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local ## L-16: Potentially missing inheritance for contract. -There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. +There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. -
15 Found Instances +
8 Found Instances -- Found in src/v0.8/functions/dev/v1_X/FunctionsBilling.sol [Line: 17](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsBilling.sol#L17) - - ```solidity - abstract contract FunctionsBilling is Routable, IFunctionsBilling { - ``` - -- Found in src/v0.8/functions/dev/v1_X/FunctionsClient.sol [Line: 11](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsClient.sol#L11) - - ```solidity - abstract contract FunctionsClient is IFunctionsClient { - ``` - - Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L13) ```solidity @@ -2686,24 +2674,12 @@ There is an interface that is potentially missing (not included in) the inherita contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` -- Found in src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol [Line: 15](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol#L15) - - ```solidity - abstract contract FunctionsSubscriptions is IFunctionsSubscriptions, IERC677Receiver { - ``` - - Found in src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol#L14) ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` -- Found in src/v0.8/functions/v1_0_0/FunctionsClient.sol [Line: 11](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsClient.sol#L11) - - ```solidity - abstract contract FunctionsClient is IFunctionsClient { - ``` - - Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L13) ```solidity @@ -2716,30 +2692,12 @@ There is an interface that is potentially missing (not included in) the inherita contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` -- Found in src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol [Line: 15](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol#L15) - - ```solidity - abstract contract FunctionsSubscriptions is IFunctionsSubscriptions, IERC677Receiver { - ``` - - Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L13) ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` -- Found in src/v0.8/functions/v1_3_0/FunctionsBilling.sol [Line: 17](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsBilling.sol#L17) - - ```solidity - abstract contract FunctionsBilling is Routable, IFunctionsBilling { - ``` - -- Found in src/v0.8/functions/v1_3_0/FunctionsClient.sol [Line: 11](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsClient.sol#L11) - - ```solidity - abstract contract FunctionsClient is IFunctionsClient { - ``` - - Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L13) ```solidity diff --git a/reports/report.json b/reports/report.json index 80e2ee3a..d1ba189d 100644 --- a/reports/report.json +++ b/reports/report.json @@ -5350,7 +5350,7 @@ }, { "title": "Potentially missing inheritance for contract.", - "description": "There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.", + "description": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.", "detector_name": "missing-inheritance", "instances": [ { diff --git a/reports/report.md b/reports/report.md index 7ce9d7fb..6fd31fcb 100644 --- a/reports/report.md +++ b/reports/report.md @@ -5454,7 +5454,7 @@ Name clashes with a built-in-symbol. Consider renaming it. ## L-35: Potentially missing inheritance for contract. -There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. +There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.
4 Found Instances diff --git a/reports/report.sarif b/reports/report.sarif index 4ac46179..182af312 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -8857,7 +8857,7 @@ } ], "message": { - "text": "There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces." + "text": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces." }, "ruleId": "missing-inheritance" } diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 35e2e340..4213c789 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8824,9 +8824,9 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local ## L-23: Potentially missing inheritance for contract. -There is an interface that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. +There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. -
6 Found Instances +
5 Found Instances - Found in contracts/amm/TreasuryIV.sol [Line: 9](../tests/2024-07-templegold/protocol/contracts/amm/TreasuryIV.sol#L9) @@ -8835,12 +8835,6 @@ There is an interface that is potentially missing (not included in) the inherita contract TreasuryIV is Ownable { ``` -- Found in contracts/core/RebasingERC20.sol [Line: 13](../tests/2024-07-templegold/protocol/contracts/core/RebasingERC20.sol#L13) - - ```solidity - abstract contract RebasingERC20 is ERC20 { - ``` - - Found in contracts/fakes/FakeERC20.sol [Line: 10](../tests/2024-07-templegold/protocol/contracts/fakes/FakeERC20.sol#L10) ```solidity From 46029fceabc607bc5da84e8fa486f66f9b7ccde8 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 24 Aug 2024 13:01:39 +0530 Subject: [PATCH 04/12] clippy --- aderyn_core/src/detect/low/missing_inheritance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index 3522e34b..69adf5f4 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -104,7 +104,7 @@ impl IssueDetector for MissingInheritanceDetector { .all(|s| contract_selectors.contains(s)) { // Now we know that `_potentially_missing_inheritance` is missing inheritance for `contract_id` - if let Some(contract) = context.nodes.get(&contract_id) { + if let Some(contract) = context.nodes.get(contract_id) { capture!(self, context, contract); } } From ef00006f8e945c7ee2d276eaaacba20283e5d60a Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 31 Aug 2024 12:02:03 +0530 Subject: [PATCH 05/12] cli/reportgen --- .../adhoc-sol-files-highs-only-report.json | 24 ++------- reports/adhoc-sol-files-report.md | 50 ++----------------- reports/sablier-aderyn-toml-nested-root.md | 10 ++-- .../src/MissingInheritance.sol | 4 +- 4 files changed, 16 insertions(+), 72 deletions(-) diff --git a/reports/adhoc-sol-files-highs-only-report.json b/reports/adhoc-sol-files-highs-only-report.json index 794c02e1..3bbb5f34 100644 --- a/reports/adhoc-sol-files-highs-only-report.json +++ b/reports/adhoc-sol-files-highs-only-report.json @@ -88,7 +88,7 @@ ] }, "issue_count": { - "high": 4, + "high": 3, "low": 0 }, "high_issues": { @@ -100,9 +100,9 @@ "instances": [ { "contract_path": "inheritance/ExtendedInheritance.sol", - "line_no": 15, - "src": "474:96", - "src_char": "474:96" + "line_no": 16, + "src": "488:19", + "src_char": "488:19" } ] }, @@ -155,19 +155,6 @@ "src_char": "391:15" } ] - }, - { - "title": "Unchecked Low level calls", - "description": "The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged.", - "detector_name": "unchecked-low-level-call", - "instances": [ - { - "contract_path": "inheritance/ExtendedInheritance.sol", - "line_no": 16, - "src": "488:71", - "src_char": "488:71" - } - ] } ] }, @@ -215,7 +202,6 @@ "incorrect-erc20-interface", "out-of-order-retryable", "constant-function-changing-state", - "function-selector-collision", - "unchecked-low-level-call" + "function-selector-collision" ] } \ No newline at end of file diff --git a/reports/adhoc-sol-files-report.md b/reports/adhoc-sol-files-report.md index d7bf3ca7..9cc7850b 100644 --- a/reports/adhoc-sol-files-report.md +++ b/reports/adhoc-sol-files-report.md @@ -11,7 +11,6 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [H-1: Using `delegatecall` in loop](#h-1-using-delegatecall-in-loop) - [H-2: Uninitialized State Variables](#h-2-uninitialized-state-variables) - [H-3: Delegatecall made by the function without checks on any adress.](#h-3-delegatecall-made-by-the-function-without-checks-on-any-adress) - - [H-4: Unchecked Low level calls](#h-4-unchecked-low-level-calls) - [Low Issues](#low-issues) - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) - [L-2: `ecrecover` is susceptible to signature malleability](#l-2-ecrecover-is-susceptible-to-signature-malleability) @@ -30,7 +29,6 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-15: Inconsistency in declaring uint256/uint (or) int256/int variables within a contract. Use explicit size declarations (uint256 or int256).](#l-15-inconsistency-in-declaring-uint256uint-or-int256int-variables-within-a-contract-use-explicit-size-declarations-uint256-or-int256) - [L-16: Unused Custom Error](#l-16-unused-custom-error) - [L-17: Potentially unused `private` / `internal` state variables found.](#l-17-potentially-unused-private--internal-state-variables-found) - - [L-18: Dead Code](#l-18-dead-code) # Summary @@ -74,8 +72,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | -| High | 4 | -| Low | 18 | +| High | 3 | +| Low | 17 | # High Issues @@ -87,10 +85,10 @@ When calling `delegatecall` the same `msg.value` amount will be accredited multi
1 Found Instances -- Found in inheritance/ExtendedInheritance.sol [Line: 15](../tests/adhoc-sol-files/inheritance/ExtendedInheritance.sol#L15) +- Found in inheritance/ExtendedInheritance.sol [Line: 16](../tests/adhoc-sol-files/inheritance/ExtendedInheritance.sol#L16) ```solidity - for (uint256 i = 0; i < 3; i++) { + target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i)); ```
@@ -155,23 +153,6 @@ Introduce checks on the address -## H-4: Unchecked Low level calls - -The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged. - -
1 Found Instances - - -- Found in inheritance/ExtendedInheritance.sol [Line: 16](../tests/adhoc-sol-files/inheritance/ExtendedInheritance.sol#L16) - - ```solidity - target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i)); - ``` - -
- - - # Low Issues ## L-1: Centralization Risk for trusted owners @@ -763,26 +744,3 @@ State variable appears to be unused. No analysis has been performed to see if an -## L-18: Dead Code - -Functions that are not used. Consider removing them. - -
2 Found Instances - - -- Found in DemoASTNodes.sol [Line: 12](../tests/adhoc-sol-files/DemoASTNodes.sol#L12) - - ```solidity - function useBreakAndContinueStatement(address x) internal pure iHaveAPlaceholder(x) returns(uint256 sum) { - ``` - -- Found in DemoASTNodes.sol [Line: 30](../tests/adhoc-sol-files/DemoASTNodes.sol#L30) - - ```solidity - function calculateSumUsingDoWhilwLoop() internal pure returns(uint256 sum) { - ``` - -
- - - diff --git a/reports/sablier-aderyn-toml-nested-root.md b/reports/sablier-aderyn-toml-nested-root.md index e5be8517..01990300 100644 --- a/reports/sablier-aderyn-toml-nested-root.md +++ b/reports/sablier-aderyn-toml-nested-root.md @@ -25,7 +25,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | | .sol Files | 20 | -| Total nSLOC | 2190 | +| Total nSLOC | 2100 | ## Files Details @@ -35,7 +35,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/SablierV2LockupDynamic.sol | 211 | | src/SablierV2LockupLinear.sol | 168 | | src/SablierV2LockupTranched.sol | 161 | -| src/SablierV2NFTDescriptor.sol | 269 | +| src/SablierV2NFTDescriptor.sol | 252 | | src/abstracts/Adminable.sol | 16 | | src/abstracts/NoDelegateCall.sol | 17 | | src/abstracts/SablierV2Lockup.sol | 391 | @@ -49,10 +49,10 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/interfaces/hooks/ISablierV2Sender.sol | 4 | | src/libraries/Errors.sol | 50 | | src/libraries/Helpers.sol | 212 | -| src/libraries/NFTSVG.sol | 144 | -| src/libraries/SVGElements.sol | 200 | +| src/libraries/NFTSVG.sol | 127 | +| src/libraries/SVGElements.sol | 144 | | src/types/DataTypes.sol | 183 | -| **Total** | **2190** | +| **Total** | **2100** | ## Issue Summary diff --git a/tests/contract-playground/src/MissingInheritance.sol b/tests/contract-playground/src/MissingInheritance.sol index b7f91772..eedb752b 100644 --- a/tests/contract-playground/src/MissingInheritance.sol +++ b/tests/contract-playground/src/MissingInheritance.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.24; +pragma solidity 0.8.19; // BAD contract MissingInheritanceCounter { @@ -23,4 +23,4 @@ contract MissingInheritanceCounter2 is IMissingInheritanceCounter { function increment() external { count += 1; } -} \ No newline at end of file +} From 98bb0423d5ae0abc072a836ec77a68d75ad79ba3 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 31 Aug 2024 12:02:58 +0530 Subject: [PATCH 06/12] cli/reportgen --- .../adhoc-sol-files-highs-only-report.json | 24 +- reports/adhoc-sol-files-report.md | 50 +- reports/ccip-functions-report.md | 60 +- reports/report.json | 478 ++++++++++- reports/report.md | 519 +++++++++++- reports/report.sarif | 786 +++++++++++++++++- reports/sablier-aderyn-toml-nested-root.md | 10 +- reports/templegold-report.md | 148 +++- 8 files changed, 1928 insertions(+), 147 deletions(-) diff --git a/reports/adhoc-sol-files-highs-only-report.json b/reports/adhoc-sol-files-highs-only-report.json index 3bbb5f34..794c02e1 100644 --- a/reports/adhoc-sol-files-highs-only-report.json +++ b/reports/adhoc-sol-files-highs-only-report.json @@ -88,7 +88,7 @@ ] }, "issue_count": { - "high": 3, + "high": 4, "low": 0 }, "high_issues": { @@ -100,9 +100,9 @@ "instances": [ { "contract_path": "inheritance/ExtendedInheritance.sol", - "line_no": 16, - "src": "488:19", - "src_char": "488:19" + "line_no": 15, + "src": "474:96", + "src_char": "474:96" } ] }, @@ -155,6 +155,19 @@ "src_char": "391:15" } ] + }, + { + "title": "Unchecked Low level calls", + "description": "The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged.", + "detector_name": "unchecked-low-level-call", + "instances": [ + { + "contract_path": "inheritance/ExtendedInheritance.sol", + "line_no": 16, + "src": "488:71", + "src_char": "488:71" + } + ] } ] }, @@ -202,6 +215,7 @@ "incorrect-erc20-interface", "out-of-order-retryable", "constant-function-changing-state", - "function-selector-collision" + "function-selector-collision", + "unchecked-low-level-call" ] } \ No newline at end of file diff --git a/reports/adhoc-sol-files-report.md b/reports/adhoc-sol-files-report.md index 9cc7850b..d7bf3ca7 100644 --- a/reports/adhoc-sol-files-report.md +++ b/reports/adhoc-sol-files-report.md @@ -11,6 +11,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [H-1: Using `delegatecall` in loop](#h-1-using-delegatecall-in-loop) - [H-2: Uninitialized State Variables](#h-2-uninitialized-state-variables) - [H-3: Delegatecall made by the function without checks on any adress.](#h-3-delegatecall-made-by-the-function-without-checks-on-any-adress) + - [H-4: Unchecked Low level calls](#h-4-unchecked-low-level-calls) - [Low Issues](#low-issues) - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) - [L-2: `ecrecover` is susceptible to signature malleability](#l-2-ecrecover-is-susceptible-to-signature-malleability) @@ -29,6 +30,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-15: Inconsistency in declaring uint256/uint (or) int256/int variables within a contract. Use explicit size declarations (uint256 or int256).](#l-15-inconsistency-in-declaring-uint256uint-or-int256int-variables-within-a-contract-use-explicit-size-declarations-uint256-or-int256) - [L-16: Unused Custom Error](#l-16-unused-custom-error) - [L-17: Potentially unused `private` / `internal` state variables found.](#l-17-potentially-unused-private--internal-state-variables-found) + - [L-18: Dead Code](#l-18-dead-code) # Summary @@ -72,8 +74,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | -| High | 3 | -| Low | 17 | +| High | 4 | +| Low | 18 | # High Issues @@ -85,10 +87,10 @@ When calling `delegatecall` the same `msg.value` amount will be accredited multi
1 Found Instances -- Found in inheritance/ExtendedInheritance.sol [Line: 16](../tests/adhoc-sol-files/inheritance/ExtendedInheritance.sol#L16) +- Found in inheritance/ExtendedInheritance.sol [Line: 15](../tests/adhoc-sol-files/inheritance/ExtendedInheritance.sol#L15) ```solidity - target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i)); + for (uint256 i = 0; i < 3; i++) { ```
@@ -153,6 +155,23 @@ Introduce checks on the address +## H-4: Unchecked Low level calls + +The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged. + +
1 Found Instances + + +- Found in inheritance/ExtendedInheritance.sol [Line: 16](../tests/adhoc-sol-files/inheritance/ExtendedInheritance.sol#L16) + + ```solidity + target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i)); + ``` + +
+ + + # Low Issues ## L-1: Centralization Risk for trusted owners @@ -744,3 +763,26 @@ State variable appears to be unused. No analysis has been performed to see if an +## L-18: Dead Code + +Functions that are not used. Consider removing them. + +
2 Found Instances + + +- Found in DemoASTNodes.sol [Line: 12](../tests/adhoc-sol-files/DemoASTNodes.sol#L12) + + ```solidity + function useBreakAndContinueStatement(address x) internal pure iHaveAPlaceholder(x) returns(uint256 sum) { + ``` + +- Found in DemoASTNodes.sol [Line: 30](../tests/adhoc-sol-files/DemoASTNodes.sol#L30) + + ```solidity + function calculateSumUsingDoWhilwLoop() internal pure returns(uint256 sum) { + ``` + +
+ + + diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 5e32b0fd..aa21912b 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -25,9 +25,10 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-11: Contract still has TODOs](#l-11-contract-still-has-todos) - [L-12: Unused Custom Error](#l-12-unused-custom-error) - [L-13: Loop contains `require`/`revert` statements](#l-13-loop-contains-requirerevert-statements) - - [L-14: Loop condition contains `state_variable.length` that could be cached outside.](#l-14-loop-condition-contains-statevariablelength-that-could-be-cached-outside) - - [L-15: Costly operations inside loops.](#l-15-costly-operations-inside-loops) - - [L-16: Potentially missing inheritance for contract.](#l-16-potentially-missing-inheritance-for-contract) + - [L-14: Dead Code](#l-14-dead-code) + - [L-15: Loop condition contains `state_variable.length` that could be cached outside.](#l-15-loop-condition-contains-statevariablelength-that-could-be-cached-outside) + - [L-16: Costly operations inside loops.](#l-16-costly-operations-inside-loops) + - [L-17: Potentially missing inheritance for contract.](#l-17-potentially-missing-inheritance-for-contract) # Summary @@ -37,7 +38,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | | .sol Files | 52 | -| Total nSLOC | 5573 | +| Total nSLOC | 5576 | ## Files Details @@ -59,7 +60,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/v0.8/functions/dev/v1_X/interfaces/IFunctionsRouter.sol | 37 | | src/v0.8/functions/dev/v1_X/interfaces/IFunctionsSubscriptions.sol | 39 | | src/v0.8/functions/dev/v1_X/interfaces/IOwnableFunctionsRouter.sol | 4 | -| src/v0.8/functions/dev/v1_X/libraries/ChainSpecificUtil.sol | 43 | +| src/v0.8/functions/dev/v1_X/libraries/ChainSpecificUtil.sol | 44 | | src/v0.8/functions/dev/v1_X/libraries/FunctionsRequest.sol | 99 | | src/v0.8/functions/dev/v1_X/libraries/FunctionsResponse.sol | 38 | | src/v0.8/functions/dev/v1_X/ocr/OCR2Abstract.sol | 42 | @@ -82,10 +83,10 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/v0.8/functions/v1_0_0/libraries/FunctionsRequest.sol | 99 | | src/v0.8/functions/v1_0_0/libraries/FunctionsResponse.sol | 38 | | src/v0.8/functions/v1_0_0/ocr/OCR2Abstract.sol | 72 | -| src/v0.8/functions/v1_0_0/ocr/OCR2Base.sol | 243 | +| src/v0.8/functions/v1_0_0/ocr/OCR2Base.sol | 244 | | src/v0.8/functions/v1_1_0/FunctionsBilling.sol | 240 | | src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol | 140 | -| src/v0.8/functions/v1_1_0/libraries/ChainSpecificUtil.sol | 43 | +| src/v0.8/functions/v1_1_0/libraries/ChainSpecificUtil.sol | 44 | | src/v0.8/functions/v1_1_0/ocr/OCR2Abstract.sol | 42 | | src/v0.8/functions/v1_1_0/ocr/OCR2Base.sol | 233 | | src/v0.8/functions/v1_3_0/FunctionsBilling.sol | 277 | @@ -96,7 +97,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/v0.8/functions/v1_3_0/interfaces/IFunctionsBilling.sol | 31 | | src/v0.8/functions/v1_3_0/ocr/OCR2Abstract.sol | 42 | | src/v0.8/functions/v1_3_0/ocr/OCR2Base.sol | 238 | -| **Total** | **5573** | +| **Total** | **5576** | ## Issue Summary @@ -104,7 +105,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 3 | -| Low | 16 | +| Low | 17 | # High Issues @@ -2453,7 +2454,42 @@ Avoid `require` / `revert` statements in a loop because a single bad item can ca -## L-14: Loop condition contains `state_variable.length` that could be cached outside. +## L-14: Dead Code + +Functions that are not used. Consider removing them. + +
4 Found Instances + + +- Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 82](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L82) + + ```solidity + function _isTransmitter(address node) internal view returns (bool) { + ``` + +- Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 81](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L81) + + ```solidity + function _isTransmitter(address node) internal view returns (bool) { + ``` + +- Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 81](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L81) + + ```solidity + function _isTransmitter(address node) internal view returns (bool) { + ``` + +- Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 83](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L83) + + ```solidity + function _isTransmitter(address node) internal view returns (bool) { + ``` + +
+ + + +## L-15: Loop condition contains `state_variable.length` that could be cached outside. Cache the lengths of storage arrays if they are used and not modified in for loops. @@ -2476,7 +2512,7 @@ Cache the lengths of storage arrays if they are used and not modified in for loo -## L-15: Costly operations inside loops. +## L-16: Costly operations inside loops. Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local variable to hold the loop computation result. @@ -2655,7 +2691,7 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local -## L-16: Potentially missing inheritance for contract. +## L-17: Potentially missing inheritance for contract. There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. diff --git a/reports/report.json b/reports/report.json index b67b1552..4743a585 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 93, - "total_sloc": 3247 + "total_source_units": 98, + "total_sloc": 3387 }, "files_details": { "files_details": [ @@ -9,6 +9,10 @@ "file_path": "src/AbstractContract.sol", "n_sloc": 11 }, + { + "file_path": "src/AderynIgnoreCustomDetectors.sol", + "n_sloc": 11 + }, { "file_path": "src/AdminContract.sol", "n_sloc": 11 @@ -93,6 +97,10 @@ "file_path": "src/DangerousUnaryOperator.sol", "n_sloc": 13 }, + { + "file_path": "src/DeadCode.sol", + "n_sloc": 23 + }, { "file_path": "src/DelegateCallWithoutAddressCheck.sol", "n_sloc": 31 @@ -129,6 +137,10 @@ "file_path": "src/FunctionInitializingState.sol", "n_sloc": 49 }, + { + "file_path": "src/FunctionPointers.sol", + "n_sloc": 10 + }, { "file_path": "src/FunctionSignatureCollision.sol", "n_sloc": 9 @@ -147,11 +159,11 @@ }, { "file_path": "src/IncorrectERC20.sol", - "n_sloc": 97 + "n_sloc": 98 }, { "file_path": "src/IncorrectERC721.sol", - "n_sloc": 231 + "n_sloc": 238 }, { "file_path": "src/IncorrectShift.sol", @@ -255,7 +267,11 @@ }, { "file_path": "src/TxOriginUsedForAuth.sol", - "n_sloc": 42 + "n_sloc": 43 + }, + { + "file_path": "src/UncheckedCalls.sol", + "n_sloc": 24 }, { "file_path": "src/UncheckedReturn.sol", @@ -265,6 +281,10 @@ "file_path": "src/UncheckedSend.sol", "n_sloc": 18 }, + { + "file_path": "src/UninitializedLocalVariables.sol", + "n_sloc": 62 + }, { "file_path": "src/UninitializedStateVariable.sol", "n_sloc": 29 @@ -327,7 +347,7 @@ }, { "file_path": "src/eth2/DepositContract.sol", - "n_sloc": 95 + "n_sloc": 96 }, { "file_path": "src/inheritance/ExtendedInheritance.sol", @@ -380,8 +400,8 @@ ] }, "issue_count": { - "high": 41, - "low": 35 + "high": 42, + "low": 38 }, "high_issues": { "issues": [ @@ -392,9 +412,9 @@ "instances": [ { "contract_path": "src/inheritance/ExtendedInheritance.sol", - "line_no": 16, - "src": "488:19", - "src_char": "488:19" + "line_no": 15, + "src": "474:96", + "src_char": "474:96" } ] }, @@ -1479,6 +1499,12 @@ "src": "145:9", "src_char": "145:9" }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 5, + "src": "93:12", + "src_char": "93:12" + }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 7, @@ -1737,12 +1763,6 @@ "src": "5072:7", "src_char": "5072:7" }, - { - "contract_path": "src/ReturnBomb.sol", - "line_no": 32, - "src": "839:4", - "src_char": "839:4" - }, { "contract_path": "src/SendEtherNoChecks.sol", "line_no": 53, @@ -1761,6 +1781,18 @@ "src": "1795:5", "src_char": "1795:5" }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 6, + "src": "99:110", + "src_char": "99:110" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 22, + "src": "572:359", + "src_char": "572:359" + }, { "contract_path": "src/UncheckedSend.sol", "line_no": 6, @@ -1804,6 +1836,18 @@ "src": "392:9", "src_char": "392:9" }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 14, + "src": "323:118", + "src_char": "323:118" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 22, + "src": "572:359", + "src_char": "572:359" + }, { "contract_path": "src/auditor_mode/ExternalCalls.sol", "line_no": 38, @@ -2342,13 +2386,106 @@ "contract_path": "src/FunctionSignatureCollision.sol", "line_no": 7, "src": "166:8", - "src_char": "166:8" + "src_char": "166:8", + "hint": "collides with the following function name(s) in scope: OwnerTransferV7b711143" }, { "contract_path": "src/FunctionSignatureCollision.sol", "line_no": 13, "src": "231:22", - "src_char": "231:22" + "src_char": "231:22", + "hint": "collides with the following function name(s) in scope: withdraw" + } + ] + }, + { + "title": "Unchecked Low level calls", + "description": "The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged.", + "detector_name": "unchecked-low-level-call", + "instances": [ + { + "contract_path": "src/DelegateCallWithoutAddressCheck.sol", + "line_no": 16, + "src": "452:21", + "src_char": "452:21" + }, + { + "contract_path": "src/DelegateCallWithoutAddressCheck.sol", + "line_no": 20, + "src": "583:26", + "src_char": "583:26" + }, + { + "contract_path": "src/DelegateCallWithoutAddressCheck.sol", + "line_no": 36, + "src": "1071:21", + "src_char": "1071:21" + }, + { + "contract_path": "src/DelegateCallWithoutAddressCheck.sol", + "line_no": 42, + "src": "1287:21", + "src_char": "1287:21" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 7, + "src": "172:30", + "src_char": "172:30" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 11, + "src": "293:17", + "src_char": "293:17" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 15, + "src": "409:25", + "src_char": "409:25" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 19, + "src": "536:23", + "src_char": "536:23" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 23, + "src": "651:27", + "src_char": "651:27" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 25, + "src": "689:66", + "src_char": "689:66" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 27, + "src": "766:86", + "src_char": "766:86" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 29, + "src": "863:61", + "src_char": "863:61" + }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 34, + "src": "1028:19", + "src_char": "1028:19" + }, + { + "contract_path": "src/inheritance/ExtendedInheritance.sol", + "line_no": 16, + "src": "488:71", + "src_char": "488:71" } ] } @@ -2732,6 +2869,12 @@ "src": "32:24", "src_char": "32:24" }, + { + "contract_path": "src/UncheckedCalls.sol", + "line_no": 2, + "src": "32:23", + "src_char": "32:23" + }, { "contract_path": "src/UncheckedSend.sol", "line_no": 2, @@ -2854,6 +2997,18 @@ "description": "Instead of marking a function as `public`, consider marking it as `external` if it is not used internally.", "detector_name": "useless-public-function", "instances": [ + { + "contract_path": "src/AderynIgnoreCustomDetectors.sol", + "line_no": 7, + "src": "174:2", + "src_char": "174:2" + }, + { + "contract_path": "src/AderynIgnoreCustomDetectors.sol", + "line_no": 26, + "src": "586:2", + "src_char": "586:2" + }, { "contract_path": "src/ArbitraryTransferFrom.sol", "line_no": 28, @@ -3046,6 +3201,24 @@ "src": "2539:25", "src_char": "2539:25" }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 7, + "src": "121:19", + "src_char": "121:19" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 23, + "src": "727:20", + "src_char": "727:20" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 41, + "src": "1527:17", + "src_char": "1527:17" + }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 17, @@ -3527,6 +3700,30 @@ "src": "1190:7", "src_char": "1190:7" }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 34, + "src": "1264:42", + "src_char": "1264:42" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 63, + "src": "2278:42", + "src_char": "2278:42" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 67, + "src": "2459:2", + "src_char": "2459:2" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 70, + "src": "2607:2", + "src_char": "2607:2" + }, { "contract_path": "src/WeakRandomness.sol", "line_no": 25, @@ -4170,6 +4367,12 @@ "description": "Consider removing empty blocks.", "detector_name": "empty-block", "instances": [ + { + "contract_path": "src/AderynIgnoreCustomDetectors.sol", + "line_no": 7, + "src": "174:2", + "src_char": "174:2" + }, { "contract_path": "src/AdminContract.sol", "line_no": 14, @@ -4714,6 +4917,42 @@ "src": "133:6", "src_char": "133:6" }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 9, + "src": "211:17", + "src_char": "211:17" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 15, + "src": "434:22", + "src_char": "434:22" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 25, + "src": "817:15", + "src_char": "817:15" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 31, + "src": "1109:20", + "src_char": "1109:20" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 43, + "src": "1639:11", + "src_char": "1639:11" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 49, + "src": "1826:16", + "src_char": "1826:16" + }, { "contract_path": "src/eth2/DepositContract.sol", "line_no": 59, @@ -4990,6 +5229,12 @@ "src": "145:9", "src_char": "145:9" }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 5, + "src": "93:12", + "src_char": "93:12" + }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 13, @@ -5138,6 +5383,103 @@ } ] }, + { + "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": 9, + "src": "211:17", + "src_char": "211:17" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 10, + "src": "243:17", + "src_char": "243:17" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 11, + "src": "278:20", + "src_char": "278:20" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 12, + "src": "312:16", + "src_char": "312:16" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 13, + "src": "346:20", + "src_char": "346:20" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 14, + "src": "390:19", + "src_char": "390:19" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 15, + "src": "434:22", + "src_char": "434:22" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 16, + "src": "481:22", + "src_char": "481:22" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 17, + "src": "531:25", + "src_char": "531:25" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 18, + "src": "580:21", + "src_char": "580:21" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 19, + "src": "629:25", + "src_char": "629:25" + }, + { + "contract_path": "src/UninitializedLocalVariables.sol", + "line_no": 20, + "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 ", @@ -5176,6 +5518,85 @@ } ] }, + { + "title": "Dead Code", + "description": "Functions that are not used. Consider removing them.", + "detector_name": "dead-code", + "instances": [ + { + "contract_path": "src/ArbitraryTransferFrom.sol", + "line_no": 15, + "src": "304:4", + "src_char": "304:4" + }, + { + "contract_path": "src/ContractLocksEther.sol", + "line_no": 54, + "src": "1540:10", + "src_char": "1540:10" + }, + { + "contract_path": "src/DeadCode.sol", + "line_no": 16, + "src": "362:22", + "src_char": "362:22" + }, + { + "contract_path": "src/DeletionNestedMappingStructureContract.sol", + "line_no": 14, + "src": "258:6", + "src_char": "258:6" + }, + { + "contract_path": "src/IncorrectShift.sol", + "line_no": 5, + "src": "100:8", + "src_char": "100:8" + }, + { + "contract_path": "src/IncorrectShift.sol", + "line_no": 13, + "src": "292:9", + "src_char": "292:9" + }, + { + "contract_path": "src/UncheckedReturn.sol", + "line_no": 12, + "src": "186:19", + "src_char": "186:19" + }, + { + "contract_path": "src/UncheckedReturn.sol", + "line_no": 20, + "src": "362:21", + "src_char": "362:21" + }, + { + "contract_path": "src/UncheckedReturn.sol", + "line_no": 25, + "src": "492:19", + "src_char": "492:19" + }, + { + "contract_path": "src/UncheckedReturn.sol", + "line_no": 30, + "src": "644:21", + "src_char": "644:21" + }, + { + "contract_path": "src/UncheckedReturn.sol", + "line_no": 35, + "src": "842:26", + "src_char": "842:26" + }, + { + "contract_path": "src/UncheckedReturn.sol", + "line_no": 40, + "src": "1049:19", + "src_char": "1049:19" + } + ] + }, { "title": "Loop condition contains `state_variable.length` that could be cached outside.", "description": "Cache the lengths of storage arrays if they are used and not modified in for loops.", @@ -5366,6 +5787,19 @@ "src_char": "104:15" } ] + }, + { + "title": "Function pointers used in constructors.", + "description": "solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors.", + "detector_name": "function-pointer-in-constructor", + "instances": [ + { + "contract_path": "src/FunctionPointers.sol", + "line_no": 13, + "src": "330:50", + "src_char": "330:50" + } + ] } ] }, @@ -5436,15 +5870,19 @@ "contract-locks-ether", "incorrect-erc721-interface", "incorrect-erc20-interface", + "uninitialized-local-variable", "return-bomb", "out-of-order-retryable", "function-initializing-state", + "dead-code", "cache-array-length", "assert-state-change", "costly-operations-inside-loops", "constant-function-changing-state", "builtin-symbol-shadow", "function-selector-collision", - "missing-inheritance" + "missing-inheritance", + "unchecked-low-level-call", + "function-pointer-in-constructor" ] } \ No newline at end of file diff --git a/reports/report.md b/reports/report.md index 4f3817c3..d034b218 100644 --- a/reports/report.md +++ b/reports/report.md @@ -49,6 +49,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [H-39: Out of order retryable transactions.](#h-39-out-of-order-retryable-transactions) - [H-40: Constant functions changing state](#h-40-constant-functions-changing-state) - [H-41: Function selector collides with other functions](#h-41-function-selector-collides-with-other-functions) + - [H-42: Unchecked Low level calls](#h-42-unchecked-low-level-calls) - [Low Issues](#low-issues) - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) - [L-2: Solmate's SafeTransferLib does not check for token contract's existence](#l-2-solmates-safetransferlib-does-not-check-for-token-contracts-existence) @@ -78,13 +79,16 @@ 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: Return Bomb](#l-29-return-bomb) - - [L-30: Function initializing state.](#l-30-function-initializing-state) - - [L-31: Loop condition contains `state_variable.length` that could be cached outside.](#l-31-loop-condition-contains-statevariablelength-that-could-be-cached-outside) - - [L-32: Incorrect use of `assert()`](#l-32-incorrect-use-of-assert) - - [L-33: Costly operations inside loops.](#l-33-costly-operations-inside-loops) - - [L-34: Builtin Symbol Shadowing](#l-34-builtin-symbol-shadowing) - - [L-35: Potentially missing inheritance for contract.](#l-35-potentially-missing-inheritance-for-contract) + - [L-29: Uninitialized local variables.](#l-29-uninitialized-local-variables) + - [L-30: Return Bomb](#l-30-return-bomb) + - [L-31: Function initializing state.](#l-31-function-initializing-state) + - [L-32: Dead Code](#l-32-dead-code) + - [L-33: Loop condition contains `state_variable.length` that could be cached outside.](#l-33-loop-condition-contains-statevariablelength-that-could-be-cached-outside) + - [L-34: Incorrect use of `assert()`](#l-34-incorrect-use-of-assert) + - [L-35: Costly operations inside loops.](#l-35-costly-operations-inside-loops) + - [L-36: Builtin Symbol Shadowing](#l-36-builtin-symbol-shadowing) + - [L-37: Potentially missing inheritance for contract.](#l-37-potentially-missing-inheritance-for-contract) + - [L-38: Function pointers used in constructors.](#l-38-function-pointers-used-in-constructors) # Summary @@ -93,8 +97,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 93 | -| Total nSLOC | 3247 | +| .sol Files | 98 | +| Total nSLOC | 3387 | ## Files Details @@ -102,6 +106,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Filepath | nSLOC | | --- | --- | | src/AbstractContract.sol | 11 | +| src/AderynIgnoreCustomDetectors.sol | 11 | | src/AdminContract.sol | 11 | | src/ArbitraryTransferFrom.sol | 37 | | src/AssemblyExample.sol | 9 | @@ -123,6 +128,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/DangerousStrictEquality1.sol | 6 | | src/DangerousStrictEquality2.sol | 9 | | src/DangerousUnaryOperator.sol | 13 | +| src/DeadCode.sol | 23 | | src/DelegateCallWithoutAddressCheck.sol | 31 | | src/DeletionNestedMappingStructureContract.sol | 11 | | src/DeprecatedOZFunctions.sol | 32 | @@ -132,12 +138,13 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/EnumerableSetIteration.sol | 55 | | src/ExperimentalEncoder.sol | 4 | | src/FunctionInitializingState.sol | 49 | +| src/FunctionPointers.sol | 10 | | src/FunctionSignatureCollision.sol | 9 | | src/HugeConstants.sol | 36 | | src/InconsistentUints.sol | 17 | | src/IncorrectCaretOperator.sol | 16 | -| src/IncorrectERC20.sol | 97 | -| src/IncorrectERC721.sol | 231 | +| src/IncorrectERC20.sol | 98 | +| src/IncorrectERC721.sol | 238 | | src/IncorrectShift.sol | 17 | | src/InternalFunctions.sol | 22 | | src/KeccakContract.sol | 21 | @@ -163,9 +170,11 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/TautologicalCompare.sol | 17 | | src/TautologyOrContradiction.sol | 11 | | src/TestERC20.sol | 62 | -| src/TxOriginUsedForAuth.sol | 42 | +| src/TxOriginUsedForAuth.sol | 43 | +| src/UncheckedCalls.sol | 24 | | src/UncheckedReturn.sol | 33 | | src/UncheckedSend.sol | 18 | +| src/UninitializedLocalVariables.sol | 62 | | src/UninitializedStateVariable.sol | 29 | | src/UnprotectedInitialize.sol | 25 | | src/UnsafeERC721Mint.sol | 18 | @@ -181,7 +190,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/cloc/AnotherHeavilyCommentedContract.sol | 32 | | src/cloc/EmptyContractFile.sol | 0 | | src/cloc/HeavilyCommentedContract.sol | 21 | -| src/eth2/DepositContract.sol | 95 | +| src/eth2/DepositContract.sol | 96 | | src/inheritance/ExtendedInheritance.sol | 17 | | src/inheritance/IContractInheritance.sol | 4 | | src/inheritance/InheritanceBase.sol | 8 | @@ -194,15 +203,15 @@ 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** | **3247** | +| **Total** | **3387** | ## Issue Summary | Category | No. of Issues | | --- | --- | -| High | 41 | -| Low | 35 | +| High | 42 | +| Low | 38 | # High Issues @@ -214,10 +223,10 @@ When calling `delegatecall` the same `msg.value` amount will be accredited multi
1 Found Instances -- Found in src/inheritance/ExtendedInheritance.sol [Line: 16](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L16) +- Found in src/inheritance/ExtendedInheritance.sol [Line: 15](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L15) ```solidity - target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i)); + for (uint256 i = 0; i < 3; i++) { ```
@@ -1224,7 +1233,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. -
32 Found Instances +
33 Found Instances - Found in src/AssemblyExample.sol [Line: 5](../tests/contract-playground/src/AssemblyExample.sol#L5) @@ -1365,6 +1374,12 @@ Solidity does initialize variables by default when you declare them, however it' uint256 y; ``` +- Found in src/UninitializedLocalVariables.sol [Line: 5](../tests/contract-playground/src/UninitializedLocalVariables.sol#L5) + + ```solidity + uint256 stateVarUint; + ``` + - Found in src/UninitializedStateVariable.sol [Line: 7](../tests/contract-playground/src/UninitializedStateVariable.sol#L7) ```solidity @@ -1590,7 +1605,7 @@ The patterns `if (… || true)` and `if (.. && false)` will always evaluate to t Introduce checks for `msg.sender` in the function -
18 Found Instances +
19 Found Instances - Found in src/CallGraphTests.sol [Line: 38](../tests/contract-playground/src/CallGraphTests.sol#L38) @@ -1647,12 +1662,6 @@ Introduce checks for `msg.sender` in the function function unstake() public { ``` -- Found in src/ReturnBomb.sol [Line: 32](../tests/contract-playground/src/ReturnBomb.sol#L32) - - ```solidity - function oops(address badGuy) public { - ``` - - Found in src/SendEtherNoChecks.sol [Line: 53](../tests/contract-playground/src/SendEtherNoChecks.sol#L53) ```solidity @@ -1671,6 +1680,18 @@ Introduce checks for `msg.sender` in the function function func1(address x) external mod1(x) { ``` +- Found in src/UncheckedCalls.sol [Line: 6](../tests/contract-playground/src/UncheckedCalls.sol#L6) + + ```solidity + function sendEther(address payable recipient) external payable { + ``` + +- Found in src/UncheckedCalls.sol [Line: 22](../tests/contract-playground/src/UncheckedCalls.sol#L22) + + ```solidity + function testMultipleUncheckedCalls(address target) external payable { + ``` + - Found in src/UncheckedSend.sol [Line: 6](../tests/contract-playground/src/UncheckedSend.sol#L6) ```solidity @@ -1709,7 +1730,7 @@ Introduce checks for `msg.sender` in the function Introduce checks on the address -
3 Found Instances +
5 Found Instances - Found in src/DelegateCallWithoutAddressCheck.sol [Line: 15](../tests/contract-playground/src/DelegateCallWithoutAddressCheck.sol#L15) @@ -1718,6 +1739,18 @@ Introduce checks on the address function delegate1(address to, bytes memory data) external { ``` +- Found in src/UncheckedCalls.sol [Line: 14](../tests/contract-playground/src/UncheckedCalls.sol#L14) + + ```solidity + function delegateCallFunction(address target, bytes calldata data) external { + ``` + +- Found in src/UncheckedCalls.sol [Line: 22](../tests/contract-playground/src/UncheckedCalls.sol#L22) + + ```solidity + function testMultipleUncheckedCalls(address target) external payable { + ``` + - Found in src/auditor_mode/ExternalCalls.sol [Line: 38](../tests/contract-playground/src/auditor_mode/ExternalCalls.sol#L38) ```solidity @@ -2328,12 +2361,14 @@ Function selector collides with other functions. This may cause the solidity fun - Found in src/FunctionSignatureCollision.sol [Line: 7](../tests/contract-playground/src/FunctionSignatureCollision.sol#L7) + collides with the following function name(s) in scope: OwnerTransferV7b711143 ```solidity function withdraw(uint256) external { ``` - Found in src/FunctionSignatureCollision.sol [Line: 13](../tests/contract-playground/src/FunctionSignatureCollision.sol#L13) + collides with the following function name(s) in scope: withdraw ```solidity function OwnerTransferV7b711143(uint256) external { ``` @@ -2342,6 +2377,101 @@ Function selector collides with other functions. This may cause the solidity fun +## H-42: Unchecked Low level calls + +The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged. + +
14 Found Instances + + +- Found in src/DelegateCallWithoutAddressCheck.sol [Line: 16](../tests/contract-playground/src/DelegateCallWithoutAddressCheck.sol#L16) + + ```solidity + to.delegatecall(data); // `to` is not protected, therefore BAD + ``` + +- Found in src/DelegateCallWithoutAddressCheck.sol [Line: 20](../tests/contract-playground/src/DelegateCallWithoutAddressCheck.sol#L20) + + ```solidity + manager.delegatecall(data); // `manager` is state variable, therefore GOOD + ``` + +- Found in src/DelegateCallWithoutAddressCheck.sol [Line: 36](../tests/contract-playground/src/DelegateCallWithoutAddressCheck.sol#L36) + + ```solidity + to.delegatecall(data); // `to` is protected, therefore GOOD + ``` + +- Found in src/DelegateCallWithoutAddressCheck.sol [Line: 42](../tests/contract-playground/src/DelegateCallWithoutAddressCheck.sol#L42) + + ```solidity + to.delegatecall(data); + ``` + +- Found in src/UncheckedCalls.sol [Line: 7](../tests/contract-playground/src/UncheckedCalls.sol#L7) + + ```solidity + recipient.call{value: 100}(""); + ``` + +- Found in src/UncheckedCalls.sol [Line: 11](../tests/contract-playground/src/UncheckedCalls.sol#L11) + + ```solidity + target.call(data); + ``` + +- Found in src/UncheckedCalls.sol [Line: 15](../tests/contract-playground/src/UncheckedCalls.sol#L15) + + ```solidity + target.delegatecall(data); + ``` + +- Found in src/UncheckedCalls.sol [Line: 19](../tests/contract-playground/src/UncheckedCalls.sol#L19) + + ```solidity + target.staticcall(data); + ``` + +- Found in src/UncheckedCalls.sol [Line: 23](../tests/contract-playground/src/UncheckedCalls.sol#L23) + + ```solidity + target.call{value: 100}(""); + ``` + +- Found in src/UncheckedCalls.sol [Line: 25](../tests/contract-playground/src/UncheckedCalls.sol#L25) + + ```solidity + target.call(abi.encodeWithSignature("someFunction(uint256)", 123)); + ``` + +- Found in src/UncheckedCalls.sol [Line: 27](../tests/contract-playground/src/UncheckedCalls.sol#L27) + + ```solidity + target.delegatecall(abi.encodeWithSignature("someOtherFunction(address)", msg.sender)); + ``` + +- Found in src/UncheckedCalls.sol [Line: 29](../tests/contract-playground/src/UncheckedCalls.sol#L29) + + ```solidity + target.staticcall(abi.encodeWithSignature("aViewFunction()")); + ``` + +- Found in src/UncheckedCalls.sol [Line: 34](../tests/contract-playground/src/UncheckedCalls.sol#L34) + + ```solidity + dst.call.value(100)(""); + ``` + +- Found in src/inheritance/ExtendedInheritance.sol [Line: 16](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L16) + + ```solidity + target.delegatecall(abi.encodeWithSignature("doSomething(uint256)", i)); + ``` + +
+ + + # Low Issues ## L-1: Centralization Risk for trusted owners @@ -2592,7 +2722,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;` -
34 Found Instances +
35 Found Instances - Found in src/BuiltinSymbolShadow.sol [Line: 2](../tests/contract-playground/src/BuiltinSymbolShadow.sol#L2) @@ -2745,6 +2875,12 @@ Consider using a specific version of Solidity in your contracts instead of a wid pragma solidity ^0.8.20; ``` +- Found in src/UncheckedCalls.sol [Line: 2](../tests/contract-playground/src/UncheckedCalls.sol#L2) + + ```solidity + pragma solidity ^0.6.0; + ``` + - Found in src/UncheckedSend.sol [Line: 2](../tests/contract-playground/src/UncheckedSend.sol#L2) ```solidity @@ -2872,8 +3008,20 @@ 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. -
46 Found Instances +
51 Found Instances + +- Found in src/AderynIgnoreCustomDetectors.sol [Line: 7](../tests/contract-playground/src/AderynIgnoreCustomDetectors.sol#L7) + + ```solidity + function f1() public { + ``` + +- Found in src/AderynIgnoreCustomDetectors.sol [Line: 26](../tests/contract-playground/src/AderynIgnoreCustomDetectors.sol#L26) + + ```solidity + function f4() public { + ``` - Found in src/ArbitraryTransferFrom.sol [Line: 28](../tests/contract-playground/src/ArbitraryTransferFrom.sol#L28) @@ -3067,6 +3215,24 @@ Instead of marking a function as `public`, consider marking it as `external` if function setNonEmptyAlteredNumbers( ``` +- Found in src/UninitializedLocalVariables.sol [Line: 7](../tests/contract-playground/src/UninitializedLocalVariables.sol#L7) + + ```solidity + function testAllDataTypesBAD() public pure { + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 23](../tests/contract-playground/src/UninitializedLocalVariables.sol#L23) + + ```solidity + function testAllDataTypesGOOD() public pure { + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 41](../tests/contract-playground/src/UninitializedLocalVariables.sol#L41) + + ```solidity + function testAllDataTypes2() public pure { + ``` + - Found in src/UninitializedStateVariable.sol [Line: 17](../tests/contract-playground/src/UninitializedStateVariable.sol#L17) ```solidity @@ -3159,7 +3325,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. -
70 Found Instances +
74 Found Instances - Found in src/AssertStateChange.sol [Line: 9](../tests/contract-playground/src/AssertStateChange.sol#L9) @@ -3534,6 +3700,30 @@ 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: 34](../tests/contract-playground/src/UninitializedLocalVariables.sol#L34) + + ```solidity + 0x0000000000000000000000000000000000000001 + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 63](../tests/contract-playground/src/UninitializedLocalVariables.sol#L63) + + ```solidity + delayedAddress = 0x0000000000000000000000000000000000000001; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 67](../tests/contract-playground/src/UninitializedLocalVariables.sol#L67) + + ```solidity + delayedUintArray[0] = 21; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 70](../tests/contract-playground/src/UninitializedLocalVariables.sol#L70) + + ```solidity + delayedIntArray[0] = -21; + ``` + - Found in src/WeakRandomness.sol [Line: 25](../tests/contract-playground/src/WeakRandomness.sol#L25) ```solidity @@ -4196,8 +4386,14 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai Consider removing empty blocks. -
33 Found Instances +
34 Found Instances + + +- Found in src/AderynIgnoreCustomDetectors.sol [Line: 7](../tests/contract-playground/src/AderynIgnoreCustomDetectors.sol#L7) + ```solidity + function f1() public { + ``` - Found in src/AdminContract.sol [Line: 14](../tests/contract-playground/src/AdminContract.sol#L14) @@ -4690,7 +4886,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. -
21 Found Instances +
27 Found Instances - Found in src/Casting.sol [Line: 31](../tests/contract-playground/src/Casting.sol#L31) @@ -4759,6 +4955,42 @@ Consider keeping the naming convention consistent in a given contract. Explicit uint x; ``` +- Found in src/UninitializedLocalVariables.sol [Line: 9](../tests/contract-playground/src/UninitializedLocalVariables.sol#L9) + + ```solidity + uint uninitializedUint; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 15](../tests/contract-playground/src/UninitializedLocalVariables.sol#L15) + + ```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: 43](../tests/contract-playground/src/UninitializedLocalVariables.sol#L43) + + ```solidity + uint delayedUint; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 49](../tests/contract-playground/src/UninitializedLocalVariables.sol#L49) + + ```solidity + uint[1] memory delayedUintArray; + ``` + - Found in src/eth2/DepositContract.sol [Line: 59](../tests/contract-playground/src/eth2/DepositContract.sol#L59) ```solidity @@ -4996,7 +5228,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. -
25 Found Instances +
26 Found Instances - Found in src/AssemblyExample.sol [Line: 5](../tests/contract-playground/src/AssemblyExample.sol#L5) @@ -5059,6 +5291,12 @@ State variable appears to be unused. No analysis has been performed to see if an uint256 y; ``` +- Found in src/UninitializedLocalVariables.sol [Line: 5](../tests/contract-playground/src/UninitializedLocalVariables.sol#L5) + + ```solidity + uint256 stateVarUint; + ``` + - Found in src/UninitializedStateVariable.sol [Line: 13](../tests/contract-playground/src/UninitializedStateVariable.sol#L13) ```solidity @@ -5217,7 +5455,108 @@ If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. -## L-29: Return Bomb +## 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: 9](../tests/contract-playground/src/UninitializedLocalVariables.sol#L9) + + ```solidity + uint uninitializedUint; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 10](../tests/contract-playground/src/UninitializedLocalVariables.sol#L10) + + ```solidity + bool uninitializedBool; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 11](../tests/contract-playground/src/UninitializedLocalVariables.sol#L11) + + ```solidity + address uninitializedAddress; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 12](../tests/contract-playground/src/UninitializedLocalVariables.sol#L12) + + ```solidity + int uninitializedInt; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 13](../tests/contract-playground/src/UninitializedLocalVariables.sol#L13) + + ```solidity + bytes32 uninitializedBytes32; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 14](../tests/contract-playground/src/UninitializedLocalVariables.sol#L14) + + ```solidity + string memory uninitializedString; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 15](../tests/contract-playground/src/UninitializedLocalVariables.sol#L15) + + ```solidity + uint[1] memory uninitializedUintArray; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 16](../tests/contract-playground/src/UninitializedLocalVariables.sol#L16) + + ```solidity + bool[1] memory uninitializedBoolArray; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 17](../tests/contract-playground/src/UninitializedLocalVariables.sol#L17) + + ```solidity + address[1] memory uninitializedAddressArray; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 18](../tests/contract-playground/src/UninitializedLocalVariables.sol#L18) + + ```solidity + int[1] memory uninitializedIntArray; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 19](../tests/contract-playground/src/UninitializedLocalVariables.sol#L19) + + ```solidity + bytes32[1] memory uninitializedBytes32Array; + ``` + +- Found in src/UninitializedLocalVariables.sol [Line: 20](../tests/contract-playground/src/UninitializedLocalVariables.sol#L20) + + ```solidity + string[1] memory uninitializedStringArray; + ``` + +
+ + + +## L-30: 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. @@ -5235,7 +5574,7 @@ A low level callee may consume all callers gas unexpectedly. Avoid unlimited imp -## L-30: Function initializing state. +## L-31: Function initializing state. Detects the immediate initialization of state variables through function calls that are not pure/constant, or that use non-constant state variable. Remove any initialization of state variables via non-constant state variables or function calls. If variables must be set upon contract deployment, locate initialization in the constructor instead. @@ -5264,7 +5603,90 @@ Detects the immediate initialization of state variables through function calls t -## L-31: Loop condition contains `state_variable.length` that could be cached outside. +## L-32: Dead Code + +Functions that are not used. Consider removing them. + +
12 Found Instances + + +- Found in src/ArbitraryTransferFrom.sol [Line: 15](../tests/contract-playground/src/ArbitraryTransferFrom.sol#L15) + + ```solidity + function bad1(address from, address to, uint256 amount) internal { + ``` + +- Found in src/ContractLocksEther.sol [Line: 54](../tests/contract-playground/src/ContractLocksEther.sol#L54) + + ```solidity + function _sendEther(address payable recipient, uint256 amount) internal { + ``` + +- Found in src/DeadCode.sol [Line: 16](../tests/contract-playground/src/DeadCode.sol#L16) + + ```solidity + function unusedInternalFunction() internal pure returns (string memory) { + ``` + +- Found in src/DeletionNestedMappingStructureContract.sol [Line: 14](../tests/contract-playground/src/DeletionNestedMappingStructureContract.sol#L14) + + ```solidity + function remove() internal{ + ``` + +- Found in src/IncorrectShift.sol [Line: 5](../tests/contract-playground/src/IncorrectShift.sol#L5) + + ```solidity + function shiftBad() internal pure returns (uint shifted) { + ``` + +- Found in src/IncorrectShift.sol [Line: 13](../tests/contract-playground/src/IncorrectShift.sol#L13) + + ```solidity + function shiftGood() internal pure returns (uint shifted) { + ``` + +- Found in src/UncheckedReturn.sol [Line: 12](../tests/contract-playground/src/UncheckedReturn.sol#L12) + + ```solidity + function callOneAndDoNothing() internal pure { + ``` + +- Found in src/UncheckedReturn.sol [Line: 20](../tests/contract-playground/src/UncheckedReturn.sol#L20) + + ```solidity + function callOneAndDoSomething() internal { + ``` + +- Found in src/UncheckedReturn.sol [Line: 25](../tests/contract-playground/src/UncheckedReturn.sol#L25) + + ```solidity + function callTwoAndDoNothing() internal pure { + ``` + +- Found in src/UncheckedReturn.sol [Line: 30](../tests/contract-playground/src/UncheckedReturn.sol#L30) + + ```solidity + function callTwoAndDoSomething() internal pure { + ``` + +- Found in src/UncheckedReturn.sol [Line: 35](../tests/contract-playground/src/UncheckedReturn.sol#L35) + + ```solidity + function callTwoAndRequireSomething() internal pure { + ``` + +- Found in src/UncheckedReturn.sol [Line: 40](../tests/contract-playground/src/UncheckedReturn.sol#L40) + + ```solidity + function callTwoAndEmitError() internal pure { + ``` + +
+ + + +## L-33: Loop condition contains `state_variable.length` that could be cached outside. Cache the lengths of storage arrays if they are used and not modified in for loops. @@ -5293,7 +5715,7 @@ Cache the lengths of storage arrays if they are used and not modified in for loo -## L-32: Incorrect use of `assert()` +## L-34: Incorrect use of `assert()` Argument to `assert()` modifies the state. Use `require` for invariants modifying state. @@ -5310,7 +5732,7 @@ Argument to `assert()` modifies the state. Use `require` for invariants modifyin -## L-33: Costly operations inside loops. +## L-35: Costly operations inside loops. Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local variable to hold the loop computation result. @@ -5405,7 +5827,7 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local -## L-34: Builtin Symbol Shadowing +## L-36: Builtin Symbol Shadowing Name clashes with a built-in-symbol. Consider renaming it. @@ -5440,7 +5862,7 @@ Name clashes with a built-in-symbol. Consider renaming it. -## L-35: Potentially missing inheritance for contract. +## L-37: Potentially missing inheritance for contract. There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. @@ -5475,3 +5897,20 @@ There is an interface / abstract contract that is potentially missing (not inclu +## L-38: Function pointers used in constructors. + +solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors. + +
1 Found Instances + + +- Found in src/FunctionPointers.sol [Line: 13](../tests/contract-playground/src/FunctionPointers.sol#L13) + + ```solidity + function(uint, uint) pure returns (uint) operation; + ``` + +
+ + + diff --git a/reports/report.sarif b/reports/report.sarif index c7562b96..f24a3f52 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -13,8 +13,8 @@ "uri": "src/inheritance/ExtendedInheritance.sol" }, "region": { - "byteLength": 19, - "byteOffset": 488 + "byteLength": 96, + "byteOffset": 474 } } } @@ -1946,6 +1946,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 12, + "byteOffset": 93 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -2399,11 +2410,11 @@ { "physicalLocation": { "artifactLocation": { - "uri": "src/ReturnBomb.sol" + "uri": "src/SendEtherNoChecks.sol" }, "region": { - "byteLength": 4, - "byteOffset": 839 + "byteLength": 5, + "byteOffset": 1060 } } }, @@ -2414,7 +2425,7 @@ }, "region": { "byteLength": 5, - "byteOffset": 1060 + "byteOffset": 1405 } } }, @@ -2425,18 +2436,29 @@ }, "region": { "byteLength": 5, - "byteOffset": 1405 + "byteOffset": 1795 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/SendEtherNoChecks.sol" + "uri": "src/UncheckedCalls.sol" }, "region": { - "byteLength": 5, - "byteOffset": 1795 + "byteLength": 110, + "byteOffset": 99 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 359, + "byteOffset": 572 } } }, @@ -2515,6 +2537,28 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 118, + "byteOffset": 323 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 359, + "byteOffset": 572 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -3426,6 +3470,9 @@ "level": "warning", "locations": [ { + "message": { + "text": "collides with the following function name(s) in scope: OwnerTransferV7b711143" + }, "physicalLocation": { "artifactLocation": { "uri": "src/FunctionSignatureCollision.sol" @@ -3437,6 +3484,9 @@ } }, { + "message": { + "text": "collides with the following function name(s) in scope: withdraw" + }, "physicalLocation": { "artifactLocation": { "uri": "src/FunctionSignatureCollision.sol" @@ -3453,6 +3503,169 @@ }, "ruleId": "function-selector-collision" }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/DelegateCallWithoutAddressCheck.sol" + }, + "region": { + "byteLength": 21, + "byteOffset": 452 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/DelegateCallWithoutAddressCheck.sol" + }, + "region": { + "byteLength": 26, + "byteOffset": 583 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/DelegateCallWithoutAddressCheck.sol" + }, + "region": { + "byteLength": 21, + "byteOffset": 1071 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/DelegateCallWithoutAddressCheck.sol" + }, + "region": { + "byteLength": 21, + "byteOffset": 1287 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 30, + "byteOffset": 172 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 17, + "byteOffset": 293 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 25, + "byteOffset": 409 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 23, + "byteOffset": 536 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 27, + "byteOffset": 651 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 66, + "byteOffset": 689 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 86, + "byteOffset": 766 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 61, + "byteOffset": 863 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 1028 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/inheritance/ExtendedInheritance.sol" + }, + "region": { + "byteLength": 71, + "byteOffset": 488 + } + } + } + ], + "message": { + "text": "The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Ensure that the return value of a low-level call is checked or logged." + }, + "ruleId": "unchecked-low-level-call" + }, { "level": "note", "locations": [ @@ -4117,6 +4330,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedCalls.sol" + }, + "region": { + "byteLength": 23, + "byteOffset": 32 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -4333,6 +4557,28 @@ { "level": "note", "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/AderynIgnoreCustomDetectors.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 174 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/AderynIgnoreCustomDetectors.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 586 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -4685,6 +4931,39 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 121 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 20, + "byteOffset": 727 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 17, + "byteOffset": 1527 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -5566,44 +5845,88 @@ { "physicalLocation": { "artifactLocation": { - "uri": "src/WeakRandomness.sol" + "uri": "src/UninitializedLocalVariables.sol" }, "region": { - "byteLength": 2, - "byteOffset": 933 + "byteLength": 42, + "byteOffset": 1264 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/WeakRandomness.sol" + "uri": "src/UninitializedLocalVariables.sol" }, "region": { - "byteLength": 2, - "byteOffset": 1252 + "byteLength": 42, + "byteOffset": 2278 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/WeakRandomness.sol" + "uri": "src/UninitializedLocalVariables.sol" }, "region": { "byteLength": 2, - "byteOffset": 1441 + "byteOffset": 2459 } } }, { "physicalLocation": { "artifactLocation": { - "uri": "src/eth2/DepositContract.sol" + "uri": "src/UninitializedLocalVariables.sol" }, "region": { "byteLength": 2, - "byteOffset": 7252 + "byteOffset": 2607 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/WeakRandomness.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 933 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/WeakRandomness.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 1252 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/WeakRandomness.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 1441 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/eth2/DepositContract.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 7252 } } }, @@ -6715,6 +7038,17 @@ { "level": "note", "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/AderynIgnoreCustomDetectors.sol" + }, + "region": { + "byteLength": 2, + "byteOffset": 174 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -7697,6 +8031,72 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 17, + "byteOffset": 211 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 22, + "byteOffset": 434 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 817 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 20, + "byteOffset": 1109 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 11, + "byteOffset": 1639 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 16, + "byteOffset": 1826 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -8180,6 +8580,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedLocalVariables.sol" + }, + "region": { + "byteLength": 12, + "byteOffset": 93 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -8446,6 +8857,180 @@ }, "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": [ @@ -8508,6 +9093,147 @@ }, "ruleId": "function-initializing-state" }, + { + "level": "note", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/ArbitraryTransferFrom.sol" + }, + "region": { + "byteLength": 4, + "byteOffset": 304 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/ContractLocksEther.sol" + }, + "region": { + "byteLength": 10, + "byteOffset": 1540 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/DeadCode.sol" + }, + "region": { + "byteLength": 22, + "byteOffset": 362 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/DeletionNestedMappingStructureContract.sol" + }, + "region": { + "byteLength": 6, + "byteOffset": 258 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectShift.sol" + }, + "region": { + "byteLength": 8, + "byteOffset": 100 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectShift.sol" + }, + "region": { + "byteLength": 9, + "byteOffset": 292 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedReturn.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 186 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedReturn.sol" + }, + "region": { + "byteLength": 21, + "byteOffset": 362 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedReturn.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 492 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedReturn.sol" + }, + "region": { + "byteLength": 21, + "byteOffset": 644 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedReturn.sol" + }, + "region": { + "byteLength": 26, + "byteOffset": 842 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedReturn.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 1049 + } + } + } + ], + "message": { + "text": "Functions that are not used. Consider removing them." + }, + "ruleId": "dead-code" + }, { "level": "note", "locations": [ @@ -8838,6 +9564,26 @@ "text": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces." }, "ruleId": "missing-inheritance" + }, + { + "level": "note", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/FunctionPointers.sol" + }, + "region": { + "byteLength": 50, + "byteOffset": 330 + } + } + } + ], + "message": { + "text": "solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors." + }, + "ruleId": "function-pointer-in-constructor" } ], "tool": { diff --git a/reports/sablier-aderyn-toml-nested-root.md b/reports/sablier-aderyn-toml-nested-root.md index 01990300..e5be8517 100644 --- a/reports/sablier-aderyn-toml-nested-root.md +++ b/reports/sablier-aderyn-toml-nested-root.md @@ -25,7 +25,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | | .sol Files | 20 | -| Total nSLOC | 2100 | +| Total nSLOC | 2190 | ## Files Details @@ -35,7 +35,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/SablierV2LockupDynamic.sol | 211 | | src/SablierV2LockupLinear.sol | 168 | | src/SablierV2LockupTranched.sol | 161 | -| src/SablierV2NFTDescriptor.sol | 252 | +| src/SablierV2NFTDescriptor.sol | 269 | | src/abstracts/Adminable.sol | 16 | | src/abstracts/NoDelegateCall.sol | 17 | | src/abstracts/SablierV2Lockup.sol | 391 | @@ -49,10 +49,10 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/interfaces/hooks/ISablierV2Sender.sol | 4 | | src/libraries/Errors.sol | 50 | | src/libraries/Helpers.sol | 212 | -| src/libraries/NFTSVG.sol | 127 | -| src/libraries/SVGElements.sol | 144 | +| src/libraries/NFTSVG.sol | 144 | +| src/libraries/SVGElements.sol | 200 | | src/types/DataTypes.sol | 183 | -| **Total** | **2100** | +| **Total** | **2190** | ## Issue Summary diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 4213c789..846b5ab3 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -40,8 +40,9 @@ 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: Costly operations inside loops.](#l-22-costly-operations-inside-loops) - - [L-23: Potentially missing inheritance for contract.](#l-23-potentially-missing-inheritance-for-contract) + - [L-22: Uninitialized local variables.](#l-22-uninitialized-local-variables) + - [L-23: Costly operations inside loops.](#l-23-costly-operations-inside-loops) + - [L-24: Potentially missing inheritance for contract.](#l-24-potentially-missing-inheritance-for-contract) # Summary @@ -51,14 +52,14 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | | .sol Files | 129 | -| Total nSLOC | 10216 | +| Total nSLOC | 10226 | ## Files Details | Filepath | nSLOC | | --- | --- | -| contracts/admin/TempleTeamPayments.sol | 80 | +| contracts/admin/TempleTeamPayments.sol | 81 | | contracts/admin/TempleTeamPaymentsFactory.sol | 113 | | contracts/admin/TempleTeamPaymentsV2.sol | 68 | | contracts/amm/TempleStableAMMRouter.sol | 174 | @@ -104,7 +105,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | contracts/deprecated/Faith.sol | 41 | | contracts/deprecated/IExitQueue.sol | 4 | | contracts/deprecated/InstantExitQueue.sol | 20 | -| contracts/deprecated/LockedOGTemple.sol | 59 | +| contracts/deprecated/LockedOGTemple.sol | 61 | | contracts/deprecated/OGTemple.sol | 13 | | contracts/deprecated/TempleStaking.sol | 98 | | contracts/fakes/FakeERC20.sol | 20 | @@ -112,14 +113,14 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | contracts/fakes/NoopLiquidator.sol | 13 | | contracts/fakes/NoopVaultedTempleLiquidator.sol | 16 | | contracts/fakes/UniswapV2Factory.sol | 2 | -| contracts/fakes/UniswapV2Router02NoEth.sol | 353 | +| contracts/fakes/UniswapV2Router02NoEth.sol | 354 | | contracts/fakes/governance/DummyTimelockController.sol | 28 | | contracts/fakes/templegold/TempleGoldMock.sol | 78 | -| contracts/fakes/templegold/TempleGoldStakingMock.sol | 435 | +| contracts/fakes/templegold/TempleGoldStakingMock.sol | 436 | | contracts/fakes/templegold/TempleTokenMock.sol | 21 | | contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol | 45 | | contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol | 116 | -| contracts/governance/ElderElection.sol | 108 | +| contracts/governance/ElderElection.sol | 109 | | contracts/governance/Templar.sol | 49 | | contracts/governance/TemplarMetadata.sol | 23 | | contracts/interfaces/amo/IAuraStaking.sol | 40 | @@ -165,7 +166,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | contracts/templegold/SpiceAuctionFactory.sol | 42 | | contracts/templegold/TempleGold.sol | 172 | | contracts/templegold/TempleGoldAdmin.sol | 50 | -| contracts/templegold/TempleGoldStaking.sol | 354 | +| contracts/templegold/TempleGoldStaking.sol | 355 | | contracts/templegold/TempleTeleporter.sol | 58 | | contracts/util/ABDKMath64x64.sol | 478 | | contracts/util/ABDKMathQuad.sol | 793 | @@ -177,7 +178,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | contracts/v2/circuitBreaker/TempleCircuitBreakerProxy.sol | 49 | | contracts/v2/interestRate/BaseInterestRateModel.sol | 16 | | contracts/v2/interestRate/CompoundedInterest.sol | 15 | -| contracts/v2/interestRate/LinearWithKinkInterestRateModel.sol | 94 | +| contracts/v2/interestRate/LinearWithKinkInterestRateModel.sol | 97 | | contracts/v2/safeGuards/SafeForked.sol | 65 | | contracts/v2/safeGuards/ThresholdSafeGuard.sol | 124 | | contracts/v2/strategies/AbstractStrategy.sol | 106 | @@ -187,7 +188,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | contracts/v2/strategies/TempleTokenBaseStrategy.sol | 62 | | contracts/v2/strategies/TlcStrategy.sol | 56 | | contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol | 468 | -| **Total** | **10216** | +| **Total** | **10226** | ## Issue Summary @@ -195,7 +196,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 10 | -| Low | 23 | +| Low | 24 | # High Issues @@ -303,7 +304,7 @@ Downcasting int/uints in Solidity can be unsafe due to the potential for data lo When compiling contracts with certain development frameworks (for example: Truffle), having contracts with the same name across different files can lead to one being overwritten. -
6 Found Instances +
4 Found Instances - Found in contracts/amo/helpers/BalancerPoolHelper.sol [Line: 13](../tests/2024-07-templegold/protocol/contracts/amo/helpers/BalancerPoolHelper.sol#L13) @@ -330,18 +331,6 @@ When compiling contracts with certain development frameworks (for example: Truff interface IBalancerHelpers { ``` -- Found in contracts/templegold//AuctionBase.sol [Line: 11](../tests/2024-07-templegold/protocol/contracts/templegold/AuctionBase.sol#L11) - - ```solidity - abstract contract AuctionBase is IAuctionBase { - ``` - -- Found in contracts/templegold/AuctionBase.sol [Line: 11](../tests/2024-07-templegold/protocol/contracts/templegold/AuctionBase.sol#L11) - - ```solidity - abstract contract AuctionBase is IAuctionBase { - ``` -
@@ -1207,7 +1196,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;` -
81 Found Instances +
80 Found Instances - Found in contracts/admin/TempleTeamPayments.sol [Line: 2](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPayments.sol#L2) @@ -1516,12 +1505,6 @@ Consider using a specific version of Solidity in your contracts instead of a wid pragma solidity ^0.8.20; ``` -- Found in contracts/templegold//AuctionBase.sol [Line: 1](../tests/2024-07-templegold/protocol/contracts/templegold/AuctionBase.sol#L1) - - ```solidity - pragma solidity ^0.8.20; - ``` - - Found in contracts/templegold/AuctionBase.sol [Line: 1](../tests/2024-07-templegold/protocol/contracts/templegold/AuctionBase.sol#L1) ```solidity @@ -7117,7 +7100,7 @@ Use descriptive reason strings or custom errors for revert paths. 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. -
127 Found Instances +
126 Found Instances - Found in contracts/admin/TempleTeamPayments.sol [Line: 2](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPayments.sol#L2) @@ -7702,12 +7685,6 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai pragma solidity 0.8.20; ``` -- Found in contracts/templegold//AuctionBase.sol [Line: 1](../tests/2024-07-templegold/protocol/contracts/templegold/AuctionBase.sol#L1) - - ```solidity - pragma solidity ^0.8.20; - ``` - - Found in contracts/templegold/AuctionBase.sol [Line: 1](../tests/2024-07-templegold/protocol/contracts/templegold/AuctionBase.sol#L1) ```solidity @@ -8703,7 +8680,96 @@ If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. -## L-22: Costly operations inside loops. +## 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) { + ``` + +
+ + + +## L-23: Costly operations inside loops. Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local variable to hold the loop computation result. @@ -8822,7 +8888,7 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local -## L-23: Potentially missing inheritance for contract. +## L-24: Potentially missing inheritance for contract. There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. From 9ac168d17c51268d28a490c71b797a50b298ce61 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 31 Aug 2024 12:21:30 +0530 Subject: [PATCH 07/12] cli/reportgen --- .../src/detect/low/missing_inheritance.rs | 17 ++++++++-- reports/ccip-functions-report.md | 8 +++++ reports/report.json | 27 ++++++++++----- reports/report.md | 21 +++++++++--- reports/report.sarif | 28 ++++++++++++++- reports/templegold-report.md | 5 +++ .../src/MissingInheritance.sol | 34 +++++++++++++++++++ 7 files changed, 124 insertions(+), 16 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index 69adf5f4..ccf65392 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -18,6 +18,7 @@ pub struct MissingInheritanceDetector { // 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>, + hints: BTreeMap<(String, usize, String), String>, } impl IssueDetector for MissingInheritanceDetector { @@ -105,7 +106,15 @@ impl IssueDetector for MissingInheritanceDetector { { // Now we know that `_potentially_missing_inheritance` is missing inheritance for `contract_id` if let Some(contract) = context.nodes.get(contract_id) { - capture!(self, context, contract); + if let Some(ASTNode::ContractDefinition(missing_contract)) = + context.nodes.get(potentially_missing_inheritance) + { + let hint = format!( + "Consider implementing the interface - {}", + missing_contract.name + ); + capture!(self, context, contract, hint); + } } } } @@ -120,6 +129,10 @@ impl IssueDetector for MissingInheritanceDetector { IssueSeverity::Low } + fn hints(&self) -> BTreeMap<(String, usize, String), String> { + self.hints.clone() + } + fn title(&self) -> String { String::from("Potentially missing inheritance for contract.") } @@ -160,7 +173,7 @@ mod missing_inheritance_tests { println!("{:#?}", detector.instances()); // assert that the detector found the correct number of instances - assert_eq!(detector.instances().len(), 1); + assert_eq!(detector.instances().len(), 2); // assert the severity is low assert_eq!( detector.severity(), diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index aa21912b..1bb9b1e1 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -2700,48 +2700,56 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L13) + Consider implementing the interface - IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/dev/v1_X/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol#L16) + Consider implementing the interface - IFunctionsRouter ```solidity contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol#L14) + Consider implementing the interface - ITermsOfServiceAllowList ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L13) + Consider implementing the interface - IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_0_0/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsRouter.sol#L16) + Consider implementing the interface - IFunctionsRouter ```solidity contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L13) + Consider implementing the interface - IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L13) + Consider implementing the interface - IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol#L14) + Consider implementing the interface - ITermsOfServiceAllowList ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` diff --git a/reports/report.json b/reports/report.json index 4743a585..73feaeba 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { "total_source_units": 98, - "total_sloc": 3387 + "total_sloc": 3409 }, "files_details": { "files_details": [ @@ -179,7 +179,7 @@ }, { "file_path": "src/MissingInheritance.sol", - "n_sloc": 17 + "n_sloc": 39 }, { "file_path": "src/MisusedBoolean.sol", @@ -5764,27 +5764,38 @@ "instances": [ { "contract_path": "src/MissingInheritance.sol", - "line_no": 5, - "src": "73:25", - "src_char": "73:25" + "line_no": 7, + "src": "83:25", + "src_char": "83:25", + "hint": "Consider implementing the interface - IMissingInheritanceCounter" + }, + { + "contract_path": "src/MissingInheritance.sol", + "line_no": 41, + "src": "754:16", + "src_char": "754:16", + "hint": "Consider implementing the interface - IMissingParent" }, { "contract_path": "src/TestERC20.sol", "line_no": 4, "src": "70:9", - "src_char": "70:9" + "src_char": "70:9", + "hint": "Consider implementing the interface - IERC20" }, { "contract_path": "src/inheritance/ExtendedInheritance.sol", "line_no": 6, "src": "99:19", - "src_char": "99:19" + "src_char": "99:19", + "hint": "Consider implementing the interface - CrazyPragma" }, { "contract_path": "src/inheritance/InheritanceBase.sol", "line_no": 6, "src": "104:15", - "src_char": "104:15" + "src_char": "104:15", + "hint": "Consider implementing the interface - CrazyPragma" } ] }, diff --git a/reports/report.md b/reports/report.md index d034b218..e07037f1 100644 --- a/reports/report.md +++ b/reports/report.md @@ -98,7 +98,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | | .sol Files | 98 | -| Total nSLOC | 3387 | +| Total nSLOC | 3409 | ## Files Details @@ -148,7 +148,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/IncorrectShift.sol | 17 | | src/InternalFunctions.sol | 22 | | src/KeccakContract.sol | 21 | -| src/MissingInheritance.sol | 17 | +| src/MissingInheritance.sol | 39 | | src/MisusedBoolean.sol | 67 | | src/MsgValueInLoop.sol | 55 | | src/MultipleConstructorSchemes.sol | 10 | @@ -203,7 +203,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** | **3387** | +| **Total** | **3409** | ## Issue Summary @@ -5866,29 +5866,40 @@ Name clashes with a built-in-symbol. Consider renaming it. There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. -
4 Found Instances +
5 Found Instances -- Found in src/MissingInheritance.sol [Line: 5](../tests/contract-playground/src/MissingInheritance.sol#L5) +- Found in src/MissingInheritance.sol [Line: 7](../tests/contract-playground/src/MissingInheritance.sol#L7) + Consider implementing the interface - IMissingInheritanceCounter ```solidity contract MissingInheritanceCounter { ``` +- Found in src/MissingInheritance.sol [Line: 41](../tests/contract-playground/src/MissingInheritance.sol#L41) + + Consider implementing the interface - IMissingChild + ```solidity + contract MissingContract2 { + ``` + - Found in src/TestERC20.sol [Line: 4](../tests/contract-playground/src/TestERC20.sol#L4) + Consider implementing the interface - IERC20 ```solidity contract TestERC20 { ``` - Found in src/inheritance/ExtendedInheritance.sol [Line: 6](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L6) + Consider implementing the interface - CrazyPragma ```solidity contract ExtendedInheritance is InheritanceBase { ``` - Found in src/inheritance/InheritanceBase.sol [Line: 6](../tests/contract-playground/src/inheritance/InheritanceBase.sol#L6) + Consider implementing the interface - CrazyPragma ```solidity contract InheritanceBase is IContractInheritance { ``` diff --git a/reports/report.sarif b/reports/report.sarif index f24a3f52..1787f697 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9516,17 +9516,37 @@ "level": "note", "locations": [ { + "message": { + "text": "Consider implementing the interface - IMissingInheritanceCounter" + }, "physicalLocation": { "artifactLocation": { "uri": "src/MissingInheritance.sol" }, "region": { "byteLength": 25, - "byteOffset": 73 + "byteOffset": 83 } } }, { + "message": { + "text": "Consider implementing the interface - IMissingChild" + }, + "physicalLocation": { + "artifactLocation": { + "uri": "src/MissingInheritance.sol" + }, + "region": { + "byteLength": 16, + "byteOffset": 754 + } + } + }, + { + "message": { + "text": "Consider implementing the interface - IERC20" + }, "physicalLocation": { "artifactLocation": { "uri": "src/TestERC20.sol" @@ -9538,6 +9558,9 @@ } }, { + "message": { + "text": "Consider implementing the interface - CrazyPragma" + }, "physicalLocation": { "artifactLocation": { "uri": "src/inheritance/ExtendedInheritance.sol" @@ -9549,6 +9572,9 @@ } }, { + "message": { + "text": "Consider implementing the interface - CrazyPragma" + }, "physicalLocation": { "artifactLocation": { "uri": "src/inheritance/InheritanceBase.sol" diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 846b5ab3..1612f43e 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8897,30 +8897,35 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in contracts/amm/TreasuryIV.sol [Line: 9](../tests/2024-07-templegold/protocol/contracts/amm/TreasuryIV.sol#L9) + Consider implementing the interface - ITreasuryIV ```solidity contract TreasuryIV is Ownable { ``` - Found in contracts/fakes/FakeERC20.sol [Line: 10](../tests/2024-07-templegold/protocol/contracts/fakes/FakeERC20.sol#L10) + Consider implementing the interface - AMO_IAuraToken ```solidity contract FakeERC20 is ERC20 { ``` - Found in contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol [Line: 7](../tests/2024-07-templegold/protocol/contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol#L7) + Consider implementing the interface - AMO_IAuraToken ```solidity contract TempleDebtTokenTestnetAdmin { ``` - Found in contracts/templegold/TempleGoldAdmin.sol [Line: 20](../tests/2024-07-templegold/protocol/contracts/templegold/TempleGoldAdmin.sol#L20) + Consider implementing the interface - IOFTCore ```solidity contract TempleGoldAdmin is ITempleGoldAdmin, TempleElevatedAccess { ``` - Found in contracts/v2/TempleDebtToken.sol [Line: 39](../tests/2024-07-templegold/protocol/contracts/v2/TempleDebtToken.sol#L39) + Consider implementing the interface - AMO_IAuraToken ```solidity contract TempleDebtToken is ITempleDebtToken, TempleElevatedAccess { ``` diff --git a/tests/contract-playground/src/MissingInheritance.sol b/tests/contract-playground/src/MissingInheritance.sol index eedb752b..7d3baa24 100644 --- a/tests/contract-playground/src/MissingInheritance.sol +++ b/tests/contract-playground/src/MissingInheritance.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.19; +// Set 1 + // BAD contract MissingInheritanceCounter { uint256 public count; @@ -24,3 +26,35 @@ contract MissingInheritanceCounter2 is IMissingInheritanceCounter { count += 1; } } + +// Set 2 + +interface IMissingParent { + function parent() external view returns (bool); +} + +interface IMissingChild is IMissingParent { + function child() external view returns (uint256); +} + +// BAD (it could have implemented IMissingChild) +contract MissingContract2 { + function child() external pure returns (bool) { + return true; + } + + function parent() external pure returns (uint256) { + return 10; + } +} + +// GOOD (it inherits IMissingChild) +contract MissingContract3 is IMissingChild { + function parent() external pure returns (bool) { + return true; + } + + function child() external pure returns (uint256) { + return 10; + } +} From 1882d12580c483feab5d25bdc52c9e7fa99234e1 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 31 Aug 2024 13:00:17 +0530 Subject: [PATCH 08/12] cli/reportgen --- .../src/detect/low/missing_inheritance.rs | 18 ++++++------------ reports/ccip-functions-report.md | 16 ++++++++-------- reports/report.json | 10 +++++----- reports/report.md | 10 +++++----- reports/report.sarif | 10 +++++----- reports/templegold-report.md | 10 +++++----- 6 files changed, 34 insertions(+), 40 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index ccf65392..f74409a9 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::collections::{BTreeMap, HashMap}; use std::convert::identity; use std::error::Error; @@ -27,13 +27,13 @@ impl IssueDetector for MissingInheritanceDetector { let mut contract_function_selectors: HashMap> = Default::default(); // Key -> Contract ID, Value -> Set of contract/interface IDs in it's heirarchy - let mut inheritance_map: HashMap> = Default::default(); + let mut inheritance_map: HashMap> = Default::default(); for contract in context.contract_definitions() { if let Some(full_contract) = &contract.linearized_base_contracts { inheritance_map .entry(contract.id) - .or_insert(BTreeSet::from_iter(full_contract.iter().copied())); + .or_insert(Vec::from_iter(full_contract.iter().copied())); for contract_node_id in full_contract { if let Some(ASTNode::ContractDefinition(contract_node)) = @@ -106,15 +106,9 @@ impl IssueDetector for MissingInheritanceDetector { { // Now we know that `_potentially_missing_inheritance` is missing inheritance for `contract_id` if let Some(contract) = context.nodes.get(contract_id) { - if let Some(ASTNode::ContractDefinition(missing_contract)) = - context.nodes.get(potentially_missing_inheritance) - { - let hint = format!( - "Consider implementing the interface - {}", - missing_contract.name - ); - capture!(self, context, contract, hint); - } + let hint = + format!("Consider implementing the interface - {} or any if it's children", c.name); + capture!(self, context, contract, hint); } } } diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 1bb9b1e1..13fb61a5 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -2700,56 +2700,56 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator + Consider implementing the interface - IFunctionsCoordinator or any if it's children ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/dev/v1_X/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol#L16) - Consider implementing the interface - IFunctionsRouter + Consider implementing the interface - IFunctionsRouter or any if it's children ```solidity contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol#L14) - Consider implementing the interface - ITermsOfServiceAllowList + Consider implementing the interface - ITermsOfServiceAllowList or any if it's children ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator + Consider implementing the interface - IFunctionsCoordinator or any if it's children ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_0_0/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsRouter.sol#L16) - Consider implementing the interface - IFunctionsRouter + Consider implementing the interface - IFunctionsRouter or any if it's children ```solidity contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator + Consider implementing the interface - IFunctionsCoordinator or any if it's children ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator + Consider implementing the interface - IFunctionsCoordinator or any if it's children ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol#L14) - Consider implementing the interface - ITermsOfServiceAllowList + Consider implementing the interface - ITermsOfServiceAllowList or any if it's children ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` diff --git a/reports/report.json b/reports/report.json index 73feaeba..a47952ae 100644 --- a/reports/report.json +++ b/reports/report.json @@ -5767,35 +5767,35 @@ "line_no": 7, "src": "83:25", "src_char": "83:25", - "hint": "Consider implementing the interface - IMissingInheritanceCounter" + "hint": "Consider implementing the interface - IMissingInheritanceCounter or any if it's children" }, { "contract_path": "src/MissingInheritance.sol", "line_no": 41, "src": "754:16", "src_char": "754:16", - "hint": "Consider implementing the interface - IMissingParent" + "hint": "Consider implementing the interface - IMissingChild or any if it's children" }, { "contract_path": "src/TestERC20.sol", "line_no": 4, "src": "70:9", "src_char": "70:9", - "hint": "Consider implementing the interface - IERC20" + "hint": "Consider implementing the interface - IERC20 or any if it's children" }, { "contract_path": "src/inheritance/ExtendedInheritance.sol", "line_no": 6, "src": "99:19", "src_char": "99:19", - "hint": "Consider implementing the interface - CrazyPragma" + "hint": "Consider implementing the interface - CrazyPragma or any if it's children" }, { "contract_path": "src/inheritance/InheritanceBase.sol", "line_no": 6, "src": "104:15", "src_char": "104:15", - "hint": "Consider implementing the interface - CrazyPragma" + "hint": "Consider implementing the interface - CrazyPragma or any if it's children" } ] }, diff --git a/reports/report.md b/reports/report.md index e07037f1..ea5510de 100644 --- a/reports/report.md +++ b/reports/report.md @@ -5871,35 +5871,35 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in src/MissingInheritance.sol [Line: 7](../tests/contract-playground/src/MissingInheritance.sol#L7) - Consider implementing the interface - IMissingInheritanceCounter + Consider implementing the interface - IMissingInheritanceCounter or any if it's children ```solidity contract MissingInheritanceCounter { ``` - Found in src/MissingInheritance.sol [Line: 41](../tests/contract-playground/src/MissingInheritance.sol#L41) - Consider implementing the interface - IMissingChild + Consider implementing the interface - IMissingParent or any if it's children ```solidity contract MissingContract2 { ``` - Found in src/TestERC20.sol [Line: 4](../tests/contract-playground/src/TestERC20.sol#L4) - Consider implementing the interface - IERC20 + Consider implementing the interface - IERC20 or any if it's children ```solidity contract TestERC20 { ``` - Found in src/inheritance/ExtendedInheritance.sol [Line: 6](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L6) - Consider implementing the interface - CrazyPragma + Consider implementing the interface - CrazyPragma or any if it's children ```solidity contract ExtendedInheritance is InheritanceBase { ``` - Found in src/inheritance/InheritanceBase.sol [Line: 6](../tests/contract-playground/src/inheritance/InheritanceBase.sol#L6) - Consider implementing the interface - CrazyPragma + Consider implementing the interface - CrazyPragma or any if it's children ```solidity contract InheritanceBase is IContractInheritance { ``` diff --git a/reports/report.sarif b/reports/report.sarif index 1787f697..2ee6188b 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9517,7 +9517,7 @@ "locations": [ { "message": { - "text": "Consider implementing the interface - IMissingInheritanceCounter" + "text": "Consider implementing the interface - IMissingInheritanceCounter or any if it's children" }, "physicalLocation": { "artifactLocation": { @@ -9531,7 +9531,7 @@ }, { "message": { - "text": "Consider implementing the interface - IMissingChild" + "text": "Consider implementing the interface - IMissingParent or any if it's children" }, "physicalLocation": { "artifactLocation": { @@ -9545,7 +9545,7 @@ }, { "message": { - "text": "Consider implementing the interface - IERC20" + "text": "Consider implementing the interface - IERC20 or any if it's children" }, "physicalLocation": { "artifactLocation": { @@ -9559,7 +9559,7 @@ }, { "message": { - "text": "Consider implementing the interface - CrazyPragma" + "text": "Consider implementing the interface - CrazyPragma or any if it's children" }, "physicalLocation": { "artifactLocation": { @@ -9573,7 +9573,7 @@ }, { "message": { - "text": "Consider implementing the interface - CrazyPragma" + "text": "Consider implementing the interface - CrazyPragma or any if it's children" }, "physicalLocation": { "artifactLocation": { diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 1612f43e..fae09af0 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8897,35 +8897,35 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in contracts/amm/TreasuryIV.sol [Line: 9](../tests/2024-07-templegold/protocol/contracts/amm/TreasuryIV.sol#L9) - Consider implementing the interface - ITreasuryIV + Consider implementing the interface - ITreasuryIV or any if it's children ```solidity contract TreasuryIV is Ownable { ``` - Found in contracts/fakes/FakeERC20.sol [Line: 10](../tests/2024-07-templegold/protocol/contracts/fakes/FakeERC20.sol#L10) - Consider implementing the interface - AMO_IAuraToken + Consider implementing the interface - AMO_IAuraToken or any if it's children ```solidity contract FakeERC20 is ERC20 { ``` - Found in contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol [Line: 7](../tests/2024-07-templegold/protocol/contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol#L7) - Consider implementing the interface - AMO_IAuraToken + Consider implementing the interface - AMO_IAuraToken or any if it's children ```solidity contract TempleDebtTokenTestnetAdmin { ``` - Found in contracts/templegold/TempleGoldAdmin.sol [Line: 20](../tests/2024-07-templegold/protocol/contracts/templegold/TempleGoldAdmin.sol#L20) - Consider implementing the interface - IOFTCore + Consider implementing the interface - IOFTCore or any if it's children ```solidity contract TempleGoldAdmin is ITempleGoldAdmin, TempleElevatedAccess { ``` - Found in contracts/v2/TempleDebtToken.sol [Line: 39](../tests/2024-07-templegold/protocol/contracts/v2/TempleDebtToken.sol#L39) - Consider implementing the interface - AMO_IAuraToken + Consider implementing the interface - AMO_IAuraToken or any if it's children ```solidity contract TempleDebtToken is ITempleDebtToken, TempleElevatedAccess { ``` From c12d4066c4bd6f900ef644c5f181f91e5592aef0 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sat, 31 Aug 2024 13:18:40 +0530 Subject: [PATCH 09/12] changed recommendation to implement the most suitable interface --- .../src/detect/low/missing_inheritance.rs | 29 ++++++++++++++----- reports/ccip-functions-report.md | 16 +++++----- reports/report.json | 10 +++---- reports/report.md | 10 +++---- reports/report.sarif | 10 +++---- reports/templegold-report.md | 10 +++---- 6 files changed, 50 insertions(+), 35 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index f74409a9..944c9542 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::convert::identity; use std::error::Error; @@ -67,6 +67,8 @@ impl IssueDetector for MissingInheritanceDetector { } } + let mut results: HashMap> = Default::default(); + for (contract_id, contract_selectors) in &contract_function_selectors { if contract_selectors.is_empty() { continue; @@ -104,18 +106,31 @@ impl IssueDetector for MissingInheritanceDetector { .iter() .all(|s| contract_selectors.contains(s)) { - // Now we know that `_potentially_missing_inheritance` is missing inheritance for `contract_id` - if let Some(contract) = context.nodes.get(contract_id) { - let hint = - format!("Consider implementing the interface - {} or any if it's children", c.name); - capture!(self, context, contract, hint); - } + results + .entry(*contract_id) + .or_default() + .insert(c.name.clone()); } } } } } + for (contract, missing_inheritances) in results { + if let Some(ASTNode::ContractDefinition(c)) = context.nodes.get(&contract) { + let missing_inheritances_vector = + missing_inheritances.iter().cloned().collect::>(); + let missing_inheritaces_string = missing_inheritances_vector.join(", "); + + let hint = format!( + "Consider implementing the most suitable of following inheritances : {}", + missing_inheritaces_string + ); + + capture!(self, context, c, hint); + } + } + Ok(!self.found_instances.is_empty()) } diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 13fb61a5..2afc6e06 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -2700,56 +2700,56 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator or any if it's children + Consider implementing the most suitable of following inheritances : IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/dev/v1_X/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol#L16) - Consider implementing the interface - IFunctionsRouter or any if it's children + Consider implementing the most suitable of following inheritances : IFunctionsRouter ```solidity contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol#L14) - Consider implementing the interface - ITermsOfServiceAllowList or any if it's children + Consider implementing the most suitable of following inheritances : ITermsOfServiceAllowList ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator or any if it's children + Consider implementing the most suitable of following inheritances : IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_0_0/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsRouter.sol#L16) - Consider implementing the interface - IFunctionsRouter or any if it's children + Consider implementing the most suitable of following inheritances : IFunctionsRouter ```solidity contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { ``` - Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator or any if it's children + Consider implementing the most suitable of following inheritances : IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L13) - Consider implementing the interface - IFunctionsCoordinator or any if it's children + Consider implementing the most suitable of following inheritances : IFunctionsCoordinator ```solidity contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { ``` - Found in src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol#L14) - Consider implementing the interface - ITermsOfServiceAllowList or any if it's children + Consider implementing the most suitable of following inheritances : ITermsOfServiceAllowList ```solidity contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { ``` diff --git a/reports/report.json b/reports/report.json index a47952ae..d78d9701 100644 --- a/reports/report.json +++ b/reports/report.json @@ -5767,35 +5767,35 @@ "line_no": 7, "src": "83:25", "src_char": "83:25", - "hint": "Consider implementing the interface - IMissingInheritanceCounter or any if it's children" + "hint": "Consider implementing the most suitable of following inheritances : IMissingInheritanceCounter" }, { "contract_path": "src/MissingInheritance.sol", "line_no": 41, "src": "754:16", "src_char": "754:16", - "hint": "Consider implementing the interface - IMissingChild or any if it's children" + "hint": "Consider implementing the most suitable of following inheritances : IMissingChild, IMissingParent" }, { "contract_path": "src/TestERC20.sol", "line_no": 4, "src": "70:9", "src_char": "70:9", - "hint": "Consider implementing the interface - IERC20 or any if it's children" + "hint": "Consider implementing the most suitable of following inheritances : IERC20" }, { "contract_path": "src/inheritance/ExtendedInheritance.sol", "line_no": 6, "src": "99:19", "src_char": "99:19", - "hint": "Consider implementing the interface - CrazyPragma or any if it's children" + "hint": "Consider implementing the most suitable of following inheritances : CrazyPragma" }, { "contract_path": "src/inheritance/InheritanceBase.sol", "line_no": 6, "src": "104:15", "src_char": "104:15", - "hint": "Consider implementing the interface - CrazyPragma or any if it's children" + "hint": "Consider implementing the most suitable of following inheritances : CrazyPragma" } ] }, diff --git a/reports/report.md b/reports/report.md index ea5510de..bfc4b74e 100644 --- a/reports/report.md +++ b/reports/report.md @@ -5871,35 +5871,35 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in src/MissingInheritance.sol [Line: 7](../tests/contract-playground/src/MissingInheritance.sol#L7) - Consider implementing the interface - IMissingInheritanceCounter or any if it's children + Consider implementing the most suitable of following inheritances : IMissingInheritanceCounter ```solidity contract MissingInheritanceCounter { ``` - Found in src/MissingInheritance.sol [Line: 41](../tests/contract-playground/src/MissingInheritance.sol#L41) - Consider implementing the interface - IMissingParent or any if it's children + Consider implementing the most suitable of following inheritances : IMissingChild, IMissingParent ```solidity contract MissingContract2 { ``` - Found in src/TestERC20.sol [Line: 4](../tests/contract-playground/src/TestERC20.sol#L4) - Consider implementing the interface - IERC20 or any if it's children + Consider implementing the most suitable of following inheritances : IERC20 ```solidity contract TestERC20 { ``` - Found in src/inheritance/ExtendedInheritance.sol [Line: 6](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L6) - Consider implementing the interface - CrazyPragma or any if it's children + Consider implementing the most suitable of following inheritances : CrazyPragma ```solidity contract ExtendedInheritance is InheritanceBase { ``` - Found in src/inheritance/InheritanceBase.sol [Line: 6](../tests/contract-playground/src/inheritance/InheritanceBase.sol#L6) - Consider implementing the interface - CrazyPragma or any if it's children + Consider implementing the most suitable of following inheritances : CrazyPragma ```solidity contract InheritanceBase is IContractInheritance { ``` diff --git a/reports/report.sarif b/reports/report.sarif index 2ee6188b..46377a4e 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9517,7 +9517,7 @@ "locations": [ { "message": { - "text": "Consider implementing the interface - IMissingInheritanceCounter or any if it's children" + "text": "Consider implementing the most suitable of following inheritances : IMissingInheritanceCounter" }, "physicalLocation": { "artifactLocation": { @@ -9531,7 +9531,7 @@ }, { "message": { - "text": "Consider implementing the interface - IMissingParent or any if it's children" + "text": "Consider implementing the most suitable of following inheritances : IMissingChild, IMissingParent" }, "physicalLocation": { "artifactLocation": { @@ -9545,7 +9545,7 @@ }, { "message": { - "text": "Consider implementing the interface - IERC20 or any if it's children" + "text": "Consider implementing the most suitable of following inheritances : IERC20" }, "physicalLocation": { "artifactLocation": { @@ -9559,7 +9559,7 @@ }, { "message": { - "text": "Consider implementing the interface - CrazyPragma or any if it's children" + "text": "Consider implementing the most suitable of following inheritances : CrazyPragma" }, "physicalLocation": { "artifactLocation": { @@ -9573,7 +9573,7 @@ }, { "message": { - "text": "Consider implementing the interface - CrazyPragma or any if it's children" + "text": "Consider implementing the most suitable of following inheritances : CrazyPragma" }, "physicalLocation": { "artifactLocation": { diff --git a/reports/templegold-report.md b/reports/templegold-report.md index fae09af0..0e394aac 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8897,35 +8897,35 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in contracts/amm/TreasuryIV.sol [Line: 9](../tests/2024-07-templegold/protocol/contracts/amm/TreasuryIV.sol#L9) - Consider implementing the interface - ITreasuryIV or any if it's children + Consider implementing the most suitable of following inheritances : ITreasuryIV ```solidity contract TreasuryIV is Ownable { ``` - Found in contracts/fakes/FakeERC20.sol [Line: 10](../tests/2024-07-templegold/protocol/contracts/fakes/FakeERC20.sol#L10) - Consider implementing the interface - AMO_IAuraToken or any if it's children + Consider implementing the most suitable of following inheritances : AMO_IAuraToken ```solidity contract FakeERC20 is ERC20 { ``` - Found in contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol [Line: 7](../tests/2024-07-templegold/protocol/contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol#L7) - Consider implementing the interface - AMO_IAuraToken or any if it's children + Consider implementing the most suitable of following inheritances : AMO_IAuraToken ```solidity contract TempleDebtTokenTestnetAdmin { ``` - Found in contracts/templegold/TempleGoldAdmin.sol [Line: 20](../tests/2024-07-templegold/protocol/contracts/templegold/TempleGoldAdmin.sol#L20) - Consider implementing the interface - IOFTCore or any if it's children + Consider implementing the most suitable of following inheritances : IOFTCore ```solidity contract TempleGoldAdmin is ITempleGoldAdmin, TempleElevatedAccess { ``` - Found in contracts/v2/TempleDebtToken.sol [Line: 39](../tests/2024-07-templegold/protocol/contracts/v2/TempleDebtToken.sol#L39) - Consider implementing the interface - AMO_IAuraToken or any if it's children + Consider implementing the most suitable of following inheritances : AMO_IAuraToken ```solidity contract TempleDebtToken is ITempleDebtToken, TempleElevatedAccess { ``` From 10bf2e15486ff45a1da6e4a333e26516d5ac51f1 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 2 Sep 2024 11:38:03 +0530 Subject: [PATCH 10/12] restricted detector to suggest inheritance on contracts with no base contracts only --- .../src/detect/low/missing_inheritance.rs | 8 +++ reports/ccip-functions-report.md | 70 +------------------ reports/report.json | 14 ---- reports/report.md | 16 +---- reports/report.sarif | 28 -------- reports/templegold-report.md | 30 +------- 6 files changed, 11 insertions(+), 155 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index 944c9542..dc0beeda 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -118,6 +118,14 @@ impl IssueDetector for MissingInheritanceDetector { for (contract, missing_inheritances) in results { if let Some(ASTNode::ContractDefinition(c)) = context.nodes.get(&contract) { + // If the contract c already has some inheritance, don't report it because we want + // to respect the developer's choice. + if c.linearized_base_contracts + .as_ref() + .is_some_and(|chain| chain.len() != 1) + { + continue; + } let missing_inheritances_vector = missing_inheritances.iter().cloned().collect::>(); let missing_inheritaces_string = missing_inheritances_vector.join(", "); diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 2afc6e06..dd76d764 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -28,7 +28,6 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-14: Dead Code](#l-14-dead-code) - [L-15: Loop condition contains `state_variable.length` that could be cached outside.](#l-15-loop-condition-contains-statevariablelength-that-could-be-cached-outside) - [L-16: Costly operations inside loops.](#l-16-costly-operations-inside-loops) - - [L-17: Potentially missing inheritance for contract.](#l-17-potentially-missing-inheritance-for-contract) # Summary @@ -105,7 +104,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 3 | -| Low | 17 | +| Low | 16 | # High Issues @@ -2691,70 +2690,3 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local -## L-17: Potentially missing inheritance for contract. - -There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. - -
8 Found Instances - - -- Found in src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol#L13) - - Consider implementing the most suitable of following inheritances : IFunctionsCoordinator - ```solidity - contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { - ``` - -- Found in src/v0.8/functions/dev/v1_X/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol#L16) - - Consider implementing the most suitable of following inheritances : IFunctionsRouter - ```solidity - contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { - ``` - -- Found in src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol#L14) - - Consider implementing the most suitable of following inheritances : ITermsOfServiceAllowList - ```solidity - contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { - ``` - -- Found in src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsCoordinator.sol#L13) - - Consider implementing the most suitable of following inheritances : IFunctionsCoordinator - ```solidity - contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { - ``` - -- Found in src/v0.8/functions/v1_0_0/FunctionsRouter.sol [Line: 16](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsRouter.sol#L16) - - Consider implementing the most suitable of following inheritances : IFunctionsRouter - ```solidity - contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable, ITypeAndVersion, ConfirmedOwner { - ``` - -- Found in src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsCoordinator.sol#L13) - - Consider implementing the most suitable of following inheritances : IFunctionsCoordinator - ```solidity - contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { - ``` - -- Found in src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol [Line: 13](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsCoordinator.sol#L13) - - Consider implementing the most suitable of following inheritances : IFunctionsCoordinator - ```solidity - contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilling { - ``` - -- Found in src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol [Line: 14](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/accessControl/TermsOfServiceAllowList.sol#L14) - - Consider implementing the most suitable of following inheritances : ITermsOfServiceAllowList - ```solidity - contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController, ITypeAndVersion, ConfirmedOwner { - ``` - -
- - - diff --git a/reports/report.json b/reports/report.json index d78d9701..acdc1706 100644 --- a/reports/report.json +++ b/reports/report.json @@ -5782,20 +5782,6 @@ "src": "70:9", "src_char": "70:9", "hint": "Consider implementing the most suitable of following inheritances : IERC20" - }, - { - "contract_path": "src/inheritance/ExtendedInheritance.sol", - "line_no": 6, - "src": "99:19", - "src_char": "99:19", - "hint": "Consider implementing the most suitable of following inheritances : CrazyPragma" - }, - { - "contract_path": "src/inheritance/InheritanceBase.sol", - "line_no": 6, - "src": "104:15", - "src_char": "104:15", - "hint": "Consider implementing the most suitable of following inheritances : CrazyPragma" } ] }, diff --git a/reports/report.md b/reports/report.md index bfc4b74e..6db3eec1 100644 --- a/reports/report.md +++ b/reports/report.md @@ -5866,7 +5866,7 @@ Name clashes with a built-in-symbol. Consider renaming it. There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. -
5 Found Instances +
3 Found Instances - Found in src/MissingInheritance.sol [Line: 7](../tests/contract-playground/src/MissingInheritance.sol#L7) @@ -5890,20 +5890,6 @@ There is an interface / abstract contract that is potentially missing (not inclu contract TestERC20 { ``` -- Found in src/inheritance/ExtendedInheritance.sol [Line: 6](../tests/contract-playground/src/inheritance/ExtendedInheritance.sol#L6) - - Consider implementing the most suitable of following inheritances : CrazyPragma - ```solidity - contract ExtendedInheritance is InheritanceBase { - ``` - -- Found in src/inheritance/InheritanceBase.sol [Line: 6](../tests/contract-playground/src/inheritance/InheritanceBase.sol#L6) - - Consider implementing the most suitable of following inheritances : CrazyPragma - ```solidity - contract InheritanceBase is IContractInheritance { - ``` -
diff --git a/reports/report.sarif b/reports/report.sarif index 46377a4e..4c1d5c02 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9556,34 +9556,6 @@ "byteOffset": 70 } } - }, - { - "message": { - "text": "Consider implementing the most suitable of following inheritances : CrazyPragma" - }, - "physicalLocation": { - "artifactLocation": { - "uri": "src/inheritance/ExtendedInheritance.sol" - }, - "region": { - "byteLength": 19, - "byteOffset": 99 - } - } - }, - { - "message": { - "text": "Consider implementing the most suitable of following inheritances : CrazyPragma" - }, - "physicalLocation": { - "artifactLocation": { - "uri": "src/inheritance/InheritanceBase.sol" - }, - "region": { - "byteLength": 15, - "byteOffset": 104 - } - } } ], "message": { diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 0e394aac..4333ccc6 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8892,22 +8892,8 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. -
5 Found Instances - - -- Found in contracts/amm/TreasuryIV.sol [Line: 9](../tests/2024-07-templegold/protocol/contracts/amm/TreasuryIV.sol#L9) - - Consider implementing the most suitable of following inheritances : ITreasuryIV - ```solidity - contract TreasuryIV is Ownable { - ``` - -- Found in contracts/fakes/FakeERC20.sol [Line: 10](../tests/2024-07-templegold/protocol/contracts/fakes/FakeERC20.sol#L10) +
1 Found Instances - Consider implementing the most suitable of following inheritances : AMO_IAuraToken - ```solidity - contract FakeERC20 is ERC20 { - ``` - Found in contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol [Line: 7](../tests/2024-07-templegold/protocol/contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol#L7) @@ -8916,20 +8902,6 @@ There is an interface / abstract contract that is potentially missing (not inclu contract TempleDebtTokenTestnetAdmin { ``` -- Found in contracts/templegold/TempleGoldAdmin.sol [Line: 20](../tests/2024-07-templegold/protocol/contracts/templegold/TempleGoldAdmin.sol#L20) - - Consider implementing the most suitable of following inheritances : IOFTCore - ```solidity - contract TempleGoldAdmin is ITempleGoldAdmin, TempleElevatedAccess { - ``` - -- Found in contracts/v2/TempleDebtToken.sol [Line: 39](../tests/2024-07-templegold/protocol/contracts/v2/TempleDebtToken.sol#L39) - - Consider implementing the most suitable of following inheritances : AMO_IAuraToken - ```solidity - contract TempleDebtToken is ITempleDebtToken, TempleElevatedAccess { - ``` -
From 71240689b3d0803a7c9a4b09f07fe9ff6133dd71 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 2 Sep 2024 11:41:41 +0530 Subject: [PATCH 11/12] changed title to be more precise and match the new changes --- aderyn_core/src/detect/low/missing_inheritance.rs | 2 +- reports/report.json | 2 +- reports/report.md | 2 +- reports/report.sarif | 2 +- reports/templegold-report.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index dc0beeda..3d21fcb2 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -155,7 +155,7 @@ impl IssueDetector for MissingInheritanceDetector { } fn description(&self) -> String { - String::from("There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.") + String::from("There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract.") } fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { diff --git a/reports/report.json b/reports/report.json index acdc1706..8623a30d 100644 --- a/reports/report.json +++ b/reports/report.json @@ -5759,7 +5759,7 @@ }, { "title": "Potentially missing inheritance for contract.", - "description": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces.", + "description": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract.", "detector_name": "missing-inheritance", "instances": [ { diff --git a/reports/report.md b/reports/report.md index 6db3eec1..0cf00dcd 100644 --- a/reports/report.md +++ b/reports/report.md @@ -5864,7 +5864,7 @@ Name clashes with a built-in-symbol. Consider renaming it. ## L-37: Potentially missing inheritance for contract. -There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. +There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract.
3 Found Instances diff --git a/reports/report.sarif b/reports/report.sarif index 4c1d5c02..361b9a04 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9559,7 +9559,7 @@ } ], "message": { - "text": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces." + "text": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract." }, "ruleId": "missing-inheritance" }, diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 4333ccc6..a9c6639b 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8890,7 +8890,7 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local ## L-24: Potentially missing inheritance for contract. -There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract. If that's not the case, consider using the same interface instead of defining multiple identical interfaces. +There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract.
1 Found Instances From 8b540a2f2169964b1047cd027b2e48eabf6f1ead Mon Sep 17 00:00:00 2001 From: Alex Roan Date: Mon, 2 Sep 2024 15:04:18 +0100 Subject: [PATCH 12/12] Updated hint --- aderyn_core/src/detect/low/missing_inheritance.rs | 2 +- reports/report.json | 6 +++--- reports/report.md | 6 +++--- reports/report.sarif | 6 +++--- reports/templegold-report.md | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/aderyn_core/src/detect/low/missing_inheritance.rs b/aderyn_core/src/detect/low/missing_inheritance.rs index 3d21fcb2..e26eb089 100644 --- a/aderyn_core/src/detect/low/missing_inheritance.rs +++ b/aderyn_core/src/detect/low/missing_inheritance.rs @@ -131,7 +131,7 @@ impl IssueDetector for MissingInheritanceDetector { let missing_inheritaces_string = missing_inheritances_vector.join(", "); let hint = format!( - "Consider implementing the most suitable of following inheritances : {}", + "Is this contract supposed to implement an interface? Consider extending one of the following: {}", missing_inheritaces_string ); diff --git a/reports/report.json b/reports/report.json index 50a4585a..dc9fb4f0 100644 --- a/reports/report.json +++ b/reports/report.json @@ -5937,21 +5937,21 @@ "line_no": 7, "src": "83:25", "src_char": "83:25", - "hint": "Consider implementing the most suitable of following inheritances : IMissingInheritanceCounter" + "hint": "Is this contract supposed to implement an interface? Consider extending one of the following: IMissingInheritanceCounter" }, { "contract_path": "src/MissingInheritance.sol", "line_no": 41, "src": "754:16", "src_char": "754:16", - "hint": "Consider implementing the most suitable of following inheritances : IMissingChild, IMissingParent" + "hint": "Is this contract supposed to implement an interface? Consider extending one of the following: IMissingChild, IMissingParent" }, { "contract_path": "src/TestERC20.sol", "line_no": 4, "src": "70:9", "src_char": "70:9", - "hint": "Consider implementing the most suitable of following inheritances : IERC20" + "hint": "Is this contract supposed to implement an interface? Consider extending one of the following: IERC20" } ] }, diff --git a/reports/report.md b/reports/report.md index 09e7240d..7ecfc172 100644 --- a/reports/report.md +++ b/reports/report.md @@ -6026,21 +6026,21 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in src/MissingInheritance.sol [Line: 7](../tests/contract-playground/src/MissingInheritance.sol#L7) - Consider implementing the most suitable of following inheritances : IMissingInheritanceCounter + Is this contract supposed to implement an interface? Consider extending one of the following: IMissingInheritanceCounter ```solidity contract MissingInheritanceCounter { ``` - Found in src/MissingInheritance.sol [Line: 41](../tests/contract-playground/src/MissingInheritance.sol#L41) - Consider implementing the most suitable of following inheritances : IMissingChild, IMissingParent + Is this contract supposed to implement an interface? Consider extending one of the following: IMissingChild, IMissingParent ```solidity contract MissingContract2 { ``` - Found in src/TestERC20.sol [Line: 4](../tests/contract-playground/src/TestERC20.sol#L4) - Consider implementing the most suitable of following inheritances : IERC20 + Is this contract supposed to implement an interface? Consider extending one of the following: IERC20 ```solidity contract TestERC20 { ``` diff --git a/reports/report.sarif b/reports/report.sarif index 375307f2..257d333f 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9755,7 +9755,7 @@ "locations": [ { "message": { - "text": "Consider implementing the most suitable of following inheritances : IMissingInheritanceCounter" + "text": "Is this contract supposed to implement an interface? Consider extending one of the following: IMissingInheritanceCounter" }, "physicalLocation": { "artifactLocation": { @@ -9769,7 +9769,7 @@ }, { "message": { - "text": "Consider implementing the most suitable of following inheritances : IMissingChild, IMissingParent" + "text": "Is this contract supposed to implement an interface? Consider extending one of the following: IMissingChild, IMissingParent" }, "physicalLocation": { "artifactLocation": { @@ -9783,7 +9783,7 @@ }, { "message": { - "text": "Consider implementing the most suitable of following inheritances : IERC20" + "text": "Is this contract supposed to implement an interface? Consider extending one of the following: IERC20" }, "physicalLocation": { "artifactLocation": { diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 07de63cb..b2be9bf2 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -8898,7 +8898,7 @@ There is an interface / abstract contract that is potentially missing (not inclu - Found in contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol [Line: 7](../tests/2024-07-templegold/protocol/contracts/fakes/v2/TempleDebtTokenTestnetAdmin.sol#L7) - Consider implementing the most suitable of following inheritances : AMO_IAuraToken + Is this contract supposed to implement an interface? Consider extending one of the following: AMO_IAuraToken ```solidity contract TempleDebtTokenTestnetAdmin { ```