Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Low Detector: Libraries and contracts should not live in same file #735

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions aderyn_core/src/detect/detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
Box::<FucntionPointerInConstructorDetector>::default(),
Box::<StateVariableCouldBeConstantDetector>::default(),
Box::<StateVariableChangesWithoutEventDetector>::default(),
Box::<LibrarySharesFileWithContractDetector>::default(),
]
}

Expand All @@ -112,6 +113,7 @@ pub fn get_all_detectors_names() -> Vec<String> {
#[derive(Debug, PartialEq, EnumString, Display)]
#[strum(serialize_all = "kebab-case")]
pub(crate) enum IssueDetectorNamePool {
LibrarySharesFileWithContract,
StateVariableChangesWithoutEvents,
MissingInheritance,
UnusedImport,
Expand Down Expand Up @@ -216,6 +218,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option<Box<dyn Iss
IssueDetectorNamePool::LocalVariableShadowing => {
Some(Box::<LocalVariableShadowingDetector>::default())
}
IssueDetectorNamePool::LibrarySharesFileWithContract => {
Some(Box::<LibrarySharesFileWithContractDetector>::default())
}
IssueDetectorNamePool::UnusedImport => Some(Box::<UnusedImportDetector>::default()),
IssueDetectorNamePool::VoidConstructor => Some(Box::<VoidConstructorDetector>::default()),
IssueDetectorNamePool::StateVariableCouldBeDeclaredConstant => {
Expand Down
103 changes: 103 additions & 0 deletions aderyn_core/src/detect/low/library_shares_file_with_contracts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
use std::collections::BTreeMap;
use std::error::Error;

use crate::ast::{ContractKind, NodeID};

use crate::capture;
use crate::context::browser::ExtractContractDefinitions;
use crate::detect::detector::IssueDetectorNamePool;
use crate::{
context::workspace_context::WorkspaceContext,
detect::detector::{IssueDetector, IssueSeverity},
};
use eyre::Result;

#[derive(Default)]
pub struct LibrarySharesFileWithContractDetector {
// 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 LibrarySharesFileWithContractDetector {
fn detect(&mut self, context: &WorkspaceContext) -> Result<bool, Box<dyn Error>> {
// When you have found an instance of the issue,
// use the following macro to add it to `found_instances`:
//
// capture!(self, context, item);
// capture!(self, context, item, "hint");

for source_unit in context.source_units() {
let all_contracts = ExtractContractDefinitions::from(source_unit).extracted;
let number_of_non_library_contracts = all_contracts
.iter()
.filter(|c| c.kind != ContractKind::Library)
.count();
if number_of_non_library_contracts > 0 {
for library_contract in all_contracts
.iter()
.filter(|c| c.kind == ContractKind::Library)
{
capture!(self, context, library_contract);
}
}
}

Ok(!self.found_instances.is_empty())
}

fn severity(&self) -> IssueSeverity {
IssueSeverity::Low
}

fn title(&self) -> String {
String::from("Library shares file with contracts.")
}

fn description(&self) -> String {
String::from("In Solidity, libraries are typically designed to be backward compatible, so they require flexible pragma versions to support a wide range of contracts. By placing libraries in separate files with floating pragmas, you ensure that they remain adaptable to different contract versions, while contracts in other files can stick to fixed pragma versions for stability.")
}

fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
self.found_instances.clone()
}

fn hints(&self) -> BTreeMap<(String, usize, String), String> {
self.hints.clone()
}

fn name(&self) -> String {
IssueDetectorNamePool::LibrarySharesFileWithContract.to_string()
}
}

#[cfg(test)]
mod library_shares_file_with_contracts_tests {
use serial_test::serial;

use crate::detect::{
detector::IssueDetector,
low::library_shares_file_with_contracts::LibrarySharesFileWithContractDetector,
};

#[test]
#[serial]
fn test_library_shares_file_with_contracts_detector() {
let context = crate::detect::test_utils::load_solidity_source_unit(
"../tests/contract-playground/src/LibrarySharesFileWithContract.sol",
);

let mut detector = LibrarySharesFileWithContractDetector::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
);
}
}
2 changes: 2 additions & 0 deletions aderyn_core/src/detect/low/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub(crate) mod function_init_state_vars;
pub(crate) mod function_pointer_in_constructor;
pub(crate) mod inconsistent_type_names;
pub(crate) mod large_literal_value;
pub(crate) mod library_shares_file_with_contracts;
pub(crate) mod local_variable_shadowing;
pub(crate) mod missing_inheritance;
pub(crate) mod non_reentrant_before_others;
Expand Down Expand Up @@ -60,6 +61,7 @@ pub use function_init_state_vars::FunctionInitializingStateDetector;
pub use function_pointer_in_constructor::FucntionPointerInConstructorDetector;
pub use inconsistent_type_names::InconsistentTypeNamesDetector;
pub use large_literal_value::LargeLiteralValueDetector;
pub use library_shares_file_with_contracts::LibrarySharesFileWithContractDetector;
pub use local_variable_shadowing::LocalVariableShadowingDetector;
pub use missing_inheritance::MissingInheritanceDetector;
pub use non_reentrant_before_others::NonReentrantBeforeOthersDetector;
Expand Down
50 changes: 46 additions & 4 deletions reports/report.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"files_summary": {
"total_source_units": 109,
"total_sloc": 3870
"total_source_units": 110,
"total_sloc": 3884
},
"files_details": {
"files_details": [
Expand Down Expand Up @@ -181,6 +181,10 @@
"file_path": "src/KeccakContract.sol",
"n_sloc": 21
},
{
"file_path": "src/LibrarySharesFileWithContract.sol",
"n_sloc": 14
},
{
"file_path": "src/LocalVariableShadow.sol",
"n_sloc": 23
Expand Down Expand Up @@ -445,7 +449,7 @@
},
"issue_count": {
"high": 42,
"low": 43
"low": 44
},
"high_issues": {
"issues": [
Expand Down Expand Up @@ -7112,6 +7116,43 @@
"src_char": "422:9"
}
]
},
{
"title": "Library shares file with contracts.",
"description": "In Solidity, libraries are typically designed to be backward compatible, so they require flexible pragma versions to support a wide range of contracts. By placing libraries in separate files with floating pragmas, you ensure that they remain adaptable to different contract versions, while contracts in other files can stick to fixed pragma versions for stability.",
"detector_name": "library-shares-file-with-contract",
"instances": [
{
"contract_path": "src/DeadCode.sol",
"line_no": 32,
"src": "732:16",
"src_char": "732:16"
},
{
"contract_path": "src/LibrarySharesFileWithContract.sol",
"line_no": 20,
"src": "655:17",
"src_char": "655:17"
},
{
"contract_path": "src/StateVariablesManipulation.sol",
"line_no": 188,
"src": "5260:21",
"src_char": "5260:21"
},
{
"contract_path": "src/Trump.sol",
"line_no": 35,
"src": "1058:8",
"src_char": "1058:8"
},
{
"contract_path": "src/UnusedError.sol",
"line_no": 4,
"src": "65:12",
"src_char": "65:12"
}
]
}
]
},
Expand Down Expand Up @@ -7200,6 +7241,7 @@
"unchecked-low-level-call",
"function-pointer-in-constructor",
"state-variable-could-be-declared-constant",
"state-variable-changes-without-events"
"state-variable-changes-without-events",
"library-shares-file-with-contract"
]
}
51 changes: 47 additions & 4 deletions reports/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
- [L-41: Function pointers used in constructors.](#l-41-function-pointers-used-in-constructors)
- [L-42: State variable could be declared constant](#l-42-state-variable-could-be-declared-constant)
- [L-43: State variable changes but no event is emitted.](#l-43-state-variable-changes-but-no-event-is-emitted)
- [L-44: Library shares file with contracts.](#l-44-library-shares-file-with-contracts)


# Summary
Expand All @@ -102,8 +103,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati

| Key | Value |
| --- | --- |
| .sol Files | 109 |
| Total nSLOC | 3870 |
| .sol Files | 110 |
| Total nSLOC | 3884 |


## Files Details
Expand Down Expand Up @@ -154,6 +155,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/LibrarySharesFileWithContract.sol | 14 |
| src/LocalVariableShadow.sol | 23 |
| src/MissingInheritance.sol | 39 |
| src/MisusedBoolean.sol | 67 |
Expand Down Expand Up @@ -219,15 +221,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** | **3870** |
| **Total** | **3884** |


## Issue Summary

| Category | No. of Issues |
| --- | --- |
| High | 42 |
| Low | 43 |
| Low | 44 |


# High Issues
Expand Down Expand Up @@ -7218,3 +7220,44 @@ State variable changes in this function but no event is emitted.



## L-44: Library shares file with contracts.

In Solidity, libraries are typically designed to be backward compatible, so they require flexible pragma versions to support a wide range of contracts. By placing libraries in separate files with floating pragmas, you ensure that they remain adaptable to different contract versions, while contracts in other files can stick to fixed pragma versions for stability.

<details><summary>5 Found Instances</summary>


- Found in src/DeadCode.sol [Line: 32](../tests/contract-playground/src/DeadCode.sol#L32)

```solidity
library DeadCodeExample3 {
```

- Found in src/LibrarySharesFileWithContract.sol [Line: 20](../tests/contract-playground/src/LibrarySharesFileWithContract.sol#L20)

```solidity
library IAmASimpleLibrary {
```

- Found in src/StateVariablesManipulation.sol [Line: 188](../tests/contract-playground/src/StateVariablesManipulation.sol#L188)

```solidity
library SVManipulationLibrary {
```

- Found in src/Trump.sol [Line: 35](../tests/contract-playground/src/Trump.sol#L35)

```solidity
library SafeMath {
```

- Found in src/UnusedError.sol [Line: 4](../tests/contract-playground/src/UnusedError.sol#L4)

```solidity
library ErrorLibrary {
```

</details>



64 changes: 64 additions & 0 deletions reports/report.sarif
Original file line number Diff line number Diff line change
Expand Up @@ -11893,6 +11893,70 @@
"text": "State variable changes in this function but no event is emitted."
},
"ruleId": "state-variable-changes-without-events"
},
{
"level": "note",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src/DeadCode.sol"
},
"region": {
"byteLength": 16,
"byteOffset": 732
}
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "src/LibrarySharesFileWithContract.sol"
},
"region": {
"byteLength": 17,
"byteOffset": 655
}
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "src/StateVariablesManipulation.sol"
},
"region": {
"byteLength": 21,
"byteOffset": 5260
}
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "src/Trump.sol"
},
"region": {
"byteLength": 8,
"byteOffset": 1058
}
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "src/UnusedError.sol"
},
"region": {
"byteLength": 12,
"byteOffset": 65
}
}
}
],
"message": {
"text": "In Solidity, libraries are typically designed to be backward compatible, so they require flexible pragma versions to support a wide range of contracts. By placing libraries in separate files with floating pragmas, you ensure that they remain adaptable to different contract versions, while contracts in other files can stick to fixed pragma versions for stability."
},
"ruleId": "library-shares-file-with-contract"
}
],
"tool": {
Expand Down
Loading
Loading