diff --git a/aderyn_core/src/detect/detector.rs b/aderyn_core/src/detect/detector.rs index 74305116a..08f24c8cd 100644 --- a/aderyn_core/src/detect/detector.rs +++ b/aderyn_core/src/detect/detector.rs @@ -74,6 +74,7 @@ pub fn get_all_issue_detectors() -> Vec> { Box::::default(), Box::::default(), Box::::default(), + Box::::default(), Box::::default(), Box::::default(), Box::::default(), @@ -149,6 +150,7 @@ pub(crate) enum IssueDetectorNamePool { WeakRandomness, PreDeclaredLocalVariableUsage, DeleteNestedMapping, + UnusedStateVariable, ConstantFunctionsAssembly, BooleanEquality, TxOriginUsedForAuth, @@ -163,6 +165,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option { + Some(Box::::default()) + } IssueDetectorNamePool::DelegateCallInLoop => { Some(Box::::default()) } diff --git a/aderyn_core/src/detect/low/mod.rs b/aderyn_core/src/detect/low/mod.rs index 7d7663e41..7e67f3e6b 100644 --- a/aderyn_core/src/detect/low/mod.rs +++ b/aderyn_core/src/detect/low/mod.rs @@ -20,6 +20,7 @@ pub(crate) mod unindexed_events; pub(crate) mod unsafe_erc20_functions; pub(crate) mod unsafe_oz_erc721_mint; pub(crate) mod unspecific_solidity_pragma; +pub(crate) mod unused_state_variable; pub(crate) mod useless_error; pub(crate) mod useless_internal_function; pub(crate) mod useless_modifier; @@ -48,6 +49,7 @@ pub use unindexed_events::UnindexedEventsDetector; pub use unsafe_erc20_functions::UnsafeERC20FunctionsDetector; pub use unsafe_oz_erc721_mint::UnsafeERC721MintDetector; pub use unspecific_solidity_pragma::UnspecificSolidityPragmaDetector; +pub use unused_state_variable::UnusedStateVariablesDetector; pub use useless_error::UselessErrorDetector; pub use useless_internal_function::UselessInternalFunctionDetector; pub use useless_modifier::UselessModifierDetector; diff --git a/aderyn_core/src/detect/low/unused_state_variable.rs b/aderyn_core/src/detect/low/unused_state_variable.rs new file mode 100644 index 000000000..e780f56c8 --- /dev/null +++ b/aderyn_core/src/detect/low/unused_state_variable.rs @@ -0,0 +1,121 @@ +use std::collections::{BTreeMap, BTreeSet}; +use std::convert::identity; +use std::error::Error; + +use crate::ast::{ASTNode, ContractKind, NodeID, NodeType, Visibility}; + +use crate::capture; +use crate::context::browser::{ + ExtractReferencedDeclarations, ExtractVariableDeclarations, GetClosestAncestorOfTypeX, +}; +use crate::detect::detector::IssueDetectorNamePool; +use crate::{ + context::workspace_context::WorkspaceContext, + detect::detector::{IssueDetector, IssueSeverity}, +}; +use eyre::Result; + +#[derive(Default)] +pub struct UnusedStateVariablesDetector { + // 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 UnusedStateVariablesDetector { + fn detect(&mut self, context: &WorkspaceContext) -> Result> { + // Collect all referencedDeclaration IDs adn StateVariableDeclarationIDs + let mut all_referenced_declarations = BTreeSet::new(); + let mut all_state_variable_declarations = BTreeSet::new(); + + for source_unit in context.source_units() { + let referenced_declarations = + ExtractReferencedDeclarations::from(source_unit).extracted; + all_referenced_declarations.extend(referenced_declarations); + let variable_declarations = ExtractVariableDeclarations::from(source_unit).extracted; + all_state_variable_declarations.extend( + variable_declarations + .into_iter() + .filter(|v| { + v.state_variable + && (v.visibility == Visibility::Private + || v.visibility == Visibility::Internal) + }) + .map(|v| v.id), + ) + } + + // Now, retain only the ones that have not been referenced + all_state_variable_declarations.retain(|v| !all_referenced_declarations.contains(v)); + + for unused_state_var_id in all_state_variable_declarations { + if let Some(node) = context.nodes.get(&unused_state_var_id) { + if let Some(ASTNode::ContractDefinition(contract)) = + node.closest_ancestor_of_type(context, NodeType::ContractDefinition) + { + // If this variable is defined inside a contract, make sure it's not an abstract contract before capturing it + if !contract.is_abstract.is_some_and(identity) + && contract.kind == ContractKind::Contract + { + capture!(self, context, node); + } + } else { + // Otherwise, just capture it ! + capture!(self, context, node); + } + } + } + + Ok(!self.found_instances.is_empty()) + } + + fn severity(&self) -> IssueSeverity { + IssueSeverity::Low + } + + fn title(&self) -> String { + String::from("Potentially unused `private` / `internal` state variables found.") + } + + fn description(&self) -> String { + String::from("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.") + } + + fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { + self.found_instances.clone() + } + + fn name(&self) -> String { + format!("{}", IssueDetectorNamePool::UnusedStateVariable) + } +} + +#[cfg(test)] +mod unused_detector_tests { + use serial_test::serial; + + use crate::detect::{ + detector::IssueDetector, low::unused_state_variable::UnusedStateVariablesDetector, + }; + + #[test] + #[serial] + fn test_unused_state_variables() { + let context = crate::detect::test_utils::load_solidity_source_unit( + "../tests/contract-playground/src/UnusedStateVariables.sol", + ); + + let mut detector = UnusedStateVariablesDetector::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(), 4); + // assert the severity is low + assert_eq!( + detector.severity(), + crate::detect::detector::IssueSeverity::Low + ); + } +} diff --git a/reports/adhoc-sol-files-report.md b/reports/adhoc-sol-files-report.md index 17dbae04d..9cc7850bf 100644 --- a/reports/adhoc-sol-files-report.md +++ b/reports/adhoc-sol-files-report.md @@ -28,6 +28,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-14: Contract still has TODOs](#l-14-contract-still-has-todos) - [L-15: Inconsistency in declaring uint256/uint (or) int256/int variables within a contract. Use explicit size declarations (uint256 or int256).](#l-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) # Summary @@ -72,7 +73,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 @@ -690,3 +691,56 @@ it is recommended that the definition be removed when custom error is unused +## L-17: Potentially unused `private` / `internal` state variables found. + +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. + +
7 Found Instances + + +- Found in InconsistentUints.sol [Line: 16](../tests/adhoc-sol-files/InconsistentUints.sol#L16) + + ```solidity + mapping(uint256 => uint other) u2uMapping; // 5 3 + ``` + +- Found in StateVariables.sol [Line: 8](../tests/adhoc-sol-files/StateVariables.sol#L8) + + ```solidity + uint256 private staticPrivateNumber; + ``` + +- Found in StateVariables.sol [Line: 9](../tests/adhoc-sol-files/StateVariables.sol#L9) + + ```solidity + uint256 internal staticInternalNumber; + ``` + +- Found in StateVariables.sol [Line: 13](../tests/adhoc-sol-files/StateVariables.sol#L13) + + ```solidity + uint256 private staticNonEmptyPrivateNumber = 1; + ``` + +- Found in StateVariables.sol [Line: 14](../tests/adhoc-sol-files/StateVariables.sol#L14) + + ```solidity + uint256 internal staticNonEmptyInternalNumber = 2; + ``` + +- Found in StateVariables.sol [Line: 28](../tests/adhoc-sol-files/StateVariables.sol#L28) + + ```solidity + uint256 private constant PRIVATE_CONSTANT = 1; + ``` + +- Found in StateVariables.sol [Line: 29](../tests/adhoc-sol-files/StateVariables.sol#L29) + + ```solidity + uint256 internal constant INTERNAL_CONSTANT = 2; + ``` + +
+ + + diff --git a/reports/report.json b/reports/report.json index f2eda5e64..56a143752 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 79, - "total_sloc": 2278 + "total_source_units": 80, + "total_sloc": 2290 }, "files_details": { "files_details": [ @@ -229,6 +229,10 @@ "file_path": "src/UnusedError.sol", "n_sloc": 19 }, + { + "file_path": "src/UnusedStateVariables.sol", + "n_sloc": 12 + }, { "file_path": "src/UsingSelfdestruct.sol", "n_sloc": 6 @@ -325,7 +329,7 @@ }, "issue_count": { "high": 36, - "low": 27 + "low": 28 }, "high_issues": { "issues": [ @@ -1375,6 +1379,36 @@ "src": "529:11", "src_char": "529:11" }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 6, + "src": "147:13", + "src_char": "147:13" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 7, + "src": "183:13", + "src_char": "183:13" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 8, + "src": "215:10", + "src_char": "215:10" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 9, + "src": "246:12", + "src_char": "246:12" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 12, + "src": "314:11", + "src_char": "314:11" + }, { "contract_path": "src/WrongOrderOfLayout.sol", "line_no": 11, @@ -2311,6 +2345,12 @@ "src": "32:23", "src_char": "32:23" }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 2, + "src": "32:24", + "src_char": "32:24" + }, { "contract_path": "src/UsingSelfdestruct.sol", "line_no": 2, @@ -3231,6 +3271,12 @@ "src": "32:23", "src_char": "32:23" }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 2, + "src": "32:24", + "src_char": "32:24" + }, { "contract_path": "src/WeakRandomness.sol", "line_no": 2, @@ -4102,6 +4148,163 @@ } ] }, + { + "title": "Potentially unused `private` / `internal` state variables found.", + "description": "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.", + "detector_name": "unused-state-variable", + "instances": [ + { + "contract_path": "src/AssemblyExample.sol", + "line_no": 5, + "src": "97:1", + "src_char": "97:1" + }, + { + "contract_path": "src/InconsistentUints.sol", + "line_no": 16, + "src": "434:10", + "src_char": "434:10" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 8, + "src": "199:19", + "src_char": "199:19" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 9, + "src": "241:20", + "src_char": "241:20" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 13, + "src": "383:27", + "src_char": "383:27" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 14, + "src": "437:28", + "src_char": "437:28" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 28, + "src": "1056:16", + "src_char": "1056:16" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 29, + "src": "1108:17", + "src_char": "1108:17" + }, + { + "contract_path": "src/TautologyOrContradiction.sol", + "line_no": 6, + "src": "133:6", + "src_char": "133:6" + }, + { + "contract_path": "src/TautologyOrContradiction.sol", + "line_no": 7, + "src": "145:9", + "src_char": "145:9" + }, + { + "contract_path": "src/UninitializedStateVariable.sol", + "line_no": 13, + "src": "503:3", + "src_char": "503:3" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 6, + "src": "147:13", + "src_char": "147:13" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 7, + "src": "183:13", + "src_char": "183:13" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 8, + "src": "215:10", + "src_char": "215:10" + }, + { + "contract_path": "src/UnusedStateVariables.sol", + "line_no": 9, + "src": "246:12", + "src_char": "246:12" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 14, + "src": "151:3", + "src_char": "151:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 16, + "src": "190:3", + "src_char": "190:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 19, + "src": "261:3", + "src_char": "261:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 22, + "src": "367:3", + "src_char": "367:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 29, + "src": "477:3", + "src_char": "477:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 14, + "src": "160:3", + "src_char": "160:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 16, + "src": "199:3", + "src_char": "199:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 19, + "src": "270:3", + "src_char": "270:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 22, + "src": "376:3", + "src_char": "376:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 29, + "src": "486:3", + "src_char": "486:3" + } + ] + }, { "title": "Functions declared `pure` / `view` but contains assembly", "description": "If the assembly code contains bugs or unintended side effects, it can lead to incorrect results or vulnerabilities, which are hard to debug and resolve, especially when the function is meant to be simple and predictable.", @@ -4219,6 +4422,7 @@ "weak-randomness", "pre-declared-local-variable-usage", "delete-nested-mapping", + "unused-state-variable", "constant-functions-assembly", "boolean-equality", "tx-origin-used-for-auth", diff --git a/reports/report.md b/reports/report.md index bf625ccd5..24a4af272 100644 --- a/reports/report.md +++ b/reports/report.md @@ -70,8 +70,9 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-23: Incorrect Order of Division and Multiplication](#l-23-incorrect-order-of-division-and-multiplication) - [L-24: Redundant statements have no effect.](#l-24-redundant-statements-have-no-effect) - [L-25: Public variables of a contract read in an external context (using `this`).](#l-25-public-variables-of-a-contract-read-in-an-external-context-using-this) - - [L-26: Functions declared `pure` / `view` but contains assembly](#l-26-functions-declared-pure--view-but-contains-assembly) - - [L-27: Boolean equality is not required.](#l-27-boolean-equality-is-not-required) + - [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) # Summary @@ -80,8 +81,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 79 | -| Total nSLOC | 2278 | +| .sol Files | 80 | +| Total nSLOC | 2290 | ## Files Details @@ -144,6 +145,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/UnprotectedInitialize.sol | 25 | | src/UnsafeERC721Mint.sol | 18 | | src/UnusedError.sol | 19 | +| src/UnusedStateVariables.sol | 12 | | src/UsingSelfdestruct.sol | 6 | | src/WeakRandomness.sol | 59 | | src/WrongOrderOfLayout.sol | 13 | @@ -167,7 +169,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** | **2278** | +| **Total** | **2290** | ## Issue Summary @@ -175,7 +177,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 36 | -| Low | 27 | +| Low | 28 | # High Issues @@ -1197,7 +1199,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. -
17 Found Instances +
22 Found Instances - Found in src/AssemblyExample.sol [Line: 5](../tests/contract-playground/src/AssemblyExample.sol#L5) @@ -1290,6 +1292,36 @@ Solidity does initialize variables by default when you declare them, however it' address destination; // BAD ``` +- Found in src/UnusedStateVariables.sol [Line: 6](../tests/contract-playground/src/UnusedStateVariables.sol#L6) + + ```solidity + uint256 internal unusedUint256; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 7](../tests/contract-playground/src/UnusedStateVariables.sol#L7) + + ```solidity + address internal unusedAddress; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 8](../tests/contract-playground/src/UnusedStateVariables.sol#L8) + + ```solidity + bool private unusedBool; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 9](../tests/contract-playground/src/UnusedStateVariables.sol#L9) + + ```solidity + string private unusedString; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 12](../tests/contract-playground/src/UnusedStateVariables.sol#L12) + + ```solidity + bytes32 public usedBytes32; // External contracts may want to interact with it by calling it as a function + ``` + - Found in src/WrongOrderOfLayout.sol [Line: 11](../tests/contract-playground/src/WrongOrderOfLayout.sol#L11) ```solidity @@ -2226,7 +2258,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;` -
24 Found Instances +
25 Found Instances - Found in src/CompilerBugStorageSignedIntegerArray.sol [Line: 2](../tests/contract-playground/src/CompilerBugStorageSignedIntegerArray.sol#L2) @@ -2331,6 +2363,12 @@ Consider using a specific version of Solidity in your contracts instead of a wid pragma solidity ^0.7.0; ``` +- Found in src/UnusedStateVariables.sol [Line: 2](../tests/contract-playground/src/UnusedStateVariables.sol#L2) + + ```solidity + pragma solidity ^0.8.20; + ``` + - Found in src/UsingSelfdestruct.sol [Line: 2](../tests/contract-playground/src/UsingSelfdestruct.sol#L2) ```solidity @@ -3160,7 +3198,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. -
31 Found Instances +
32 Found Instances - Found in src/AdminContract.sol [Line: 2](../tests/contract-playground/src/AdminContract.sol#L2) @@ -3271,6 +3309,12 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai pragma solidity 0.8.20; ``` +- Found in src/UnusedStateVariables.sol [Line: 2](../tests/contract-playground/src/UnusedStateVariables.sol#L2) + + ```solidity + pragma solidity ^0.8.20; + ``` + - Found in src/WeakRandomness.sol [Line: 2](../tests/contract-playground/src/WeakRandomness.sol#L2) ```solidity @@ -4188,7 +4232,168 @@ The contract reads it's own variable using `this` which adds an unnecessary STAT -## L-26: Functions declared `pure` / `view` but contains assembly +## L-26: Potentially unused `private` / `internal` state variables found. + +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 + + +- Found in src/AssemblyExample.sol [Line: 5](../tests/contract-playground/src/AssemblyExample.sol#L5) + + ```solidity + uint b; + ``` + +- Found in src/InconsistentUints.sol [Line: 16](../tests/contract-playground/src/InconsistentUints.sol#L16) + + ```solidity + mapping(uint256 => uint other) u2uMapping; // 5 3 + ``` + +- Found in src/StateVariables.sol [Line: 8](../tests/contract-playground/src/StateVariables.sol#L8) + + ```solidity + uint256 private staticPrivateNumber; + ``` + +- Found in src/StateVariables.sol [Line: 9](../tests/contract-playground/src/StateVariables.sol#L9) + + ```solidity + uint256 internal staticInternalNumber; + ``` + +- Found in src/StateVariables.sol [Line: 13](../tests/contract-playground/src/StateVariables.sol#L13) + + ```solidity + uint256 private staticNonEmptyPrivateNumber = 1; + ``` + +- Found in src/StateVariables.sol [Line: 14](../tests/contract-playground/src/StateVariables.sol#L14) + + ```solidity + uint256 internal staticNonEmptyInternalNumber = 2; + ``` + +- Found in src/StateVariables.sol [Line: 28](../tests/contract-playground/src/StateVariables.sol#L28) + + ```solidity + uint256 private constant PRIVATE_CONSTANT = 1; + ``` + +- Found in src/StateVariables.sol [Line: 29](../tests/contract-playground/src/StateVariables.sol#L29) + + ```solidity + uint256 internal constant INTERNAL_CONSTANT = 2; + ``` + +- Found in src/TautologyOrContradiction.sol [Line: 6](../tests/contract-playground/src/TautologyOrContradiction.sol#L6) + + ```solidity + uint x; + ``` + +- Found in src/TautologyOrContradiction.sol [Line: 7](../tests/contract-playground/src/TautologyOrContradiction.sol#L7) + + ```solidity + uint256 y; + ``` + +- Found in src/UninitializedStateVariable.sol [Line: 13](../tests/contract-playground/src/UninitializedStateVariable.sol#L13) + + ```solidity + mapping(uint256 => uint256[]) private map; // GOOD + ``` + +- Found in src/UnusedStateVariables.sol [Line: 6](../tests/contract-playground/src/UnusedStateVariables.sol#L6) + + ```solidity + uint256 internal unusedUint256; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 7](../tests/contract-playground/src/UnusedStateVariables.sol#L7) + + ```solidity + address internal unusedAddress; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 8](../tests/contract-playground/src/UnusedStateVariables.sol#L8) + + ```solidity + bool private unusedBool; + ``` + +- Found in src/UnusedStateVariables.sol [Line: 9](../tests/contract-playground/src/UnusedStateVariables.sol#L9) + + ```solidity + string private unusedString; + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 14](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L14) + + ```solidity + uint256 s_1 = 0; + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 16](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L16) + + ```solidity + uint256 s_2 = 0; + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 19](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L19) + + ```solidity + uint256 s_3 = 0; // this is a side comment + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 22](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L22) + + ```solidity + uint256 s_4 = 0; // scc-dblah + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 29](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L29) + + ```solidity + this is longer comment */ uint256 s_5 = 0; + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 14](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L14) + + ```solidity + uint256 s_1 = 0; + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 16](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L16) + + ```solidity + uint256 s_2 = 0; + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 19](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L19) + + ```solidity + uint256 s_3 = 0; // this is a side comment + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 22](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L22) + + ```solidity + uint256 s_4 = 0; // scc-dblah + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 29](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L29) + + ```solidity + this is longer comment */ uint256 s_5 = 0; + ``` + +
+ + + +## L-27: Functions declared `pure` / `view` but contains assembly If the assembly code contains bugs or unintended side effects, it can lead to incorrect results or vulnerabilities, which are hard to debug and resolve, especially when the function is meant to be simple and predictable. @@ -4217,7 +4422,7 @@ If the assembly code contains bugs or unintended side effects, it can lead to in -## L-27: Boolean equality is not required. +## L-28: Boolean equality is not required. If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. Just use `if(x)` and `if(!x)` respectively. diff --git a/reports/report.sarif b/reports/report.sarif index 89ab1bb2e..814ded107 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -1858,6 +1858,61 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 13, + "byteOffset": 147 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 13, + "byteOffset": 183 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 10, + "byteOffset": 215 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 12, + "byteOffset": 246 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 11, + "byteOffset": 314 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -3467,6 +3522,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 24, + "byteOffset": 32 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -5123,6 +5189,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 24, + "byteOffset": 32 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -6680,6 +6757,290 @@ }, "ruleId": "public-variable-read-in-external-context" }, + { + "level": "note", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/AssemblyExample.sol" + }, + "region": { + "byteLength": 1, + "byteOffset": 97 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/InconsistentUints.sol" + }, + "region": { + "byteLength": 10, + "byteOffset": 434 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 199 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 20, + "byteOffset": 241 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 27, + "byteOffset": 383 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 28, + "byteOffset": 437 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 16, + "byteOffset": 1056 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 17, + "byteOffset": 1108 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/TautologyOrContradiction.sol" + }, + "region": { + "byteLength": 6, + "byteOffset": 133 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/TautologyOrContradiction.sol" + }, + "region": { + "byteLength": 9, + "byteOffset": 145 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedStateVariable.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 503 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 13, + "byteOffset": 147 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 13, + "byteOffset": 183 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 10, + "byteOffset": 215 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UnusedStateVariables.sol" + }, + "region": { + "byteLength": 12, + "byteOffset": 246 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 151 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 190 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 261 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 367 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 477 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 160 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 199 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 270 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 376 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 486 + } + } + } + ], + "message": { + "text": "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." + }, + "ruleId": "unused-state-variable" + }, { "level": "note", "locations": [ diff --git a/reports/templegold-report.md b/reports/templegold-report.md index bb3f1c2ac..65d230eae 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -38,7 +38,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-17: Loop contains `require`/`revert` statements](#l-17-loop-contains-requirerevert-statements) - [L-18: Incorrect Order of Division and Multiplication](#l-18-incorrect-order-of-division-and-multiplication) - [L-19: Redundant statements have no effect.](#l-19-redundant-statements-have-no-effect) - - [L-20: Boolean equality is not required.](#l-20-boolean-equality-is-not-required) + - [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) # Summary @@ -192,7 +193,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 10 | -| Low | 20 | +| Low | 21 | # High Issues @@ -8654,7 +8655,30 @@ Remove the redundant statements because no code will be generated and it just co -## L-20: Boolean equality is not required. +## L-20: Potentially unused `private` / `internal` state variables found. + +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. + +
2 Found Instances + + +- Found in contracts/fakes/templegold/TempleGoldStakingMock.sol [Line: 71](../tests/2024-07-templegold/protocol/contracts/fakes/templegold/TempleGoldStakingMock.sol#L71) + + ```solidity + mapping(address delegate => uint256 balance) private _delegateBalances; + ``` + +- Found in contracts/fakes/templegold/TempleGoldStakingMock.sol [Line: 79](../tests/2024-07-templegold/protocol/contracts/fakes/templegold/TempleGoldStakingMock.sol#L79) + + ```solidity + mapping(address account => EnumerableSet.UintSet indexes) private _accountStakes; + ``` + +
+ + + +## L-21: Boolean equality is not required. If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. Just use `if(x)` and `if(!x)` respectively. diff --git a/tests/contract-playground/src/UnusedStateVariables.sol b/tests/contract-playground/src/UnusedStateVariables.sol new file mode 100644 index 000000000..50a2387fb --- /dev/null +++ b/tests/contract-playground/src/UnusedStateVariables.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +contract UnusedStateVariables { + // Unused state variables (BAD) + uint256 internal unusedUint256; + address internal unusedAddress; + bool private unusedBool; + string private unusedString; + + // Used state variable (GOOD) + bytes32 public usedBytes32; // External contracts may want to interact with it by calling it as a function + uint256 internal usedUint256; + + function setValue(uint256 v) external { + usedUint256 = v; + } +}