Reduce fps pess strange setter

docs/strange_setter.md

@@ -6,15 +6,15 @@
 * Confidence: `Medium`
 
 ## Description
-The detector sees if a contract contains a setter (also constructor) that does not change contract storage variables.
-Setter functions MUST change the values of storage variables.
-Setter functions that do not modify storage variables may lead to contract misfunctions.
+The detector sees if a contract contains a setter (also constructor) that does not change contract storage variables or does not perform external calls using provided arguments.
+Setter functions MUST change the values of storage variables or perform external calls using provided parameters.
+Setter functions that do not use provided variables may lead to contract misfunctions.
 
 ### Potential Improvement
-Remove highlights of mappings.
+Detect shadowing before storage update/external call
 
 ## Vulnerable Scenario
 [test scenario](../tests/strange_setter_test.sol)
 
 ## Recommendation
-Make sure that setter functions modify the states of storage variables.
+Make sure that setter functions modify the states of storage variables or performs external call using provided arguments.

slitherin/detectors/strange_setter.py

@@ -2,11 +2,12 @@
 from slither.utils.output import Output
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
 from slither.core.declarations import Function
+from slither.analyses.data_dependency.data_dependency import is_dependent, is_tainted
 
 
 class StrangeSetter(AbstractDetector):
  """
- Sees if contract contains a setter, that does not change contract storage variables.
+ Sees if contract contains a setter, that does not change contract storage variables or that does not use arguments for an external call.
  """
 
  ARGUMENT = "pess-strange-setter" # slither will launch the detector with slither.py --detect mydetector
@@ -18,7 +19,7 @@ class StrangeSetter(AbstractDetector):
  "https://github.com/pessimistic-io/slitherin/blob/master/docs/strange_setter.md"
  )
  WIKI_TITLE = "Strange Setter"
- WIKI_DESCRIPTION = "Setter must write to storage variables"
+ WIKI_DESCRIPTION = "Setter must write to storage variables or pass arguments to external calls"
  WIKI_EXPLOIT_SCENARIO = "-"
  WIKI_RECOMMENDATION = "Make sure that your setter actually sets something"
 
@@ -30,6 +31,7 @@ def _is_strange_setter(self, fun: Function) -> bool:
  if not fun.parameters:
  # nothing is in the params, so we don't care
  return False
+ used_params = set()
  for (
  fin
  ) in fun.internal_calls: # branch with for-loop for setters in internal calls
@@ -39,13 +41,45 @@ def _is_strange_setter(self, fun: Function) -> bool:
  if n.state_variables_written and str(param) in str(
  n
  ): # check if there's a state variable setter using function parameters
- return False
+ used_params.add(param)
  for param in fun.parameters:
  if fun.state_variables_written:
  for n in fun.nodes:
  if str(param) in str(n):
- return False
- return True
+ used_params.add(param)
+ for param in fun.parameters:
+ for external in fun.external_calls_as_expressions:
+ for arg in [*external.arguments, external._called._expression]:
+ if str(arg) == str(param):
+ used_params.add(param)
+ intersection_len = len(set(fun.parameters) & used_params)
+ print(intersection_len, len(fun.parameters), fun.name)
+ return intersection_len != len(fun.parameters)
+
+ # def _is_strange_setter(self, fun: Function) -> bool:
+ # """Checks if setter sets smth to a storage variable and if function parameters are used when setting"""
+ # if not isinstance(fun, Function):
+ # return True
+
+ # if not fun.parameters:
+ # # nothing is in the params, so we don't care
+ # return False
+ # for (
+ # fin
+ # ) in fun.internal_calls: # branch with for-loop for setters in internal calls
+ # if isinstance(fin, Function):
+ # for param in fin.parameters:
+ # for n in fin.nodes:
+ # if n.state_variables_written and str(param) in str(
+ # n
+ # ): # check if there's a state variable setter using function parameters
+ # return False
+ # for param in fun.parameters:
+ # if fun.state_variables_written:
+ # for n in fun.nodes:
+ # if str(param) in str(n):
+ # return False
+ # return True
 
  def _is_strange_constructor(self, fun: Function) -> bool:
  """Checks if constructor sets nothing"""

tests/strange_setter_test.sol

@@ -1,5 +1,8 @@
 pragma solidity ^0.8.0;
 
+interface ExternalContract {
+ function set(bool arg) external;
+}
 // What it should detect:
 // If smth is set in the function, and the function contains parameters,
 // and this parameters were not uset to set.
@@ -37,10 +40,18 @@ contract StrangeSetter {
  }
 
  function setWithInt(bytes32 nameHash, address builder) public onlyOwner {
- uint256 x = 0; //TODO this is not detected. There are params which are not used
+ uint256 x = 0;
  vulnurable_internal(x);
  }
 
+ function setSwapEnabledExternal_ok(ExternalContract target, bool swapEnabled) external onlyOwner {
+ target.set(swapEnabled);
+ }
+
+ function setUseOnlyOneArg_vulnerable(uint256 arg1, bool isProtectedArg) external onlyOwner {
+ isProtected = isProtectedArg;
+ }
+
  function set_ok(uint256 setter) public onlyOwner {
  toSet = setter;
  }