Merge pull request #2419 from crytic/fix/fp-unused-state-var

fix: unused state var detector for abstract/library

slither/detectors/variables/unused_state_variables.py

@@ -46,8 +46,18 @@ def detect_unused(contract: Contract) -> Optional[List[StateVariable]]:
     variables_used = [item for sublist in variables_used for item in sublist]
     variables_used = list(set(variables_used + array_candidates))
 
+    # If the contract is abstract, only consider private variables as other visibilities may be used in dependencies
+    if contract.is_abstract:
+        return [
+            x
+            for x in contract.state_variables
+            if x not in variables_used and x.visibility == "private"
+        ]
+
     # Return the variables unused that are not public
-    return [x for x in contract.variables if x not in variables_used and x.visibility != "public"]
+    return [
+        x for x in contract.state_variables if x not in variables_used and x.visibility != "public"
+    ]
 
 
 class UnusedStateVars(AbstractDetector):
@@ -71,7 +81,7 @@ def _detect(self) -> List[Output]:
         """Detect unused state variables"""
         results = []
         for c in self.compilation_unit.contracts_derived:
-            if c.is_signature_only():
+            if c.is_signature_only() or c.is_library:
                 continue
             unusedVars = detect_unused(c)
             if unusedVars:

tests/e2e/detectors/snapshots/detectors__detector_UnusedStateVars_0_7_6_unused_state_sol__0.txt

@@ -4,5 +4,9 @@ A.unused4 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#7)
 
 A.unused2 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#5) is never used in B (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#11-16)
 
+H.bad1 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#38) is never used in I (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#41-46)
+
+F.bad1 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#27) is never used in F (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#26-33)
+
 A.unused3 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#6) is never used in B (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#11-16)
 

tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol

@@ -14,3 +14,32 @@ contract B is A{
         used = address(0);
     }
 }
+
+library C {
+    uint internal constant good = 0x00; // other contract can access this constant
+    function c() public pure returns (uint){
+        return 0;
+    }
+
+}
+
+abstract contract F {
+    uint private bad1 = 0x00;
+    uint private good1 = 0x00;
+    uint internal good2 = 0x00;
+    function use() external returns (uint){
+        return good1;
+    }
+}
+
+abstract contract H {
+    uint private good1 = 0x00;
+    uint internal good2 = 0x00;
+    uint internal bad1 = 0x00;
+}
+
+contract I is H {
+    function use2() external returns (uint){
+        return good2;
+    }
+}