Merge branch 'develop' into curve-vyper-version-detector

docs/price_manipulation.md

@@ -0,0 +1,15 @@
+# Price Manipulation through token transfers
+
+## Configuration
+* Check: `pess-price-manipulation`
+* Severity: `High`
+* Confidence: `Low`
+
+## Description
+The detector finds calculations that depend on the balance and supply of some token. Such calculations could be manipulated through direct transfers to the contract, increasing its balance.
+
+## Vulnerable Scenario
+[test scenario](../tests/price_manipulation_test.sol)
+
+## Recommendation
+Avoid possible manipulations of calculations because of external transfers.

slitherin/__init__.py

@@ -33,6 +33,7 @@
 from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow
 from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy
 from slitherin.detectors.vyper.reentrancy_curve_vyper_version import CurveVyperReentrancy
+from slitherin.detectors.price_manipulation import PriceManipulationDetector
 
 artbitrum_detectors = [
  ArbitrumPrevrandaoDifficulty,
@@ -66,7 +67,8 @@
  AAVEFlashloanCallbackDetector,
  PotentialArithmOverflow,
  CurveReadonlyReentrancy,
- CurveVyperReentrancy
+ CurveVyperReentrancy,
+ PriceManipulationDetector
 ]
 plugin_printers = []
 

slitherin/detectors/price_manipulation.py

@@ -0,0 +1,71 @@
+from typing import List
+from slither.utils.output import Output
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.slithir.operations.high_level_call import HighLevelCall
+from slither.slithir.operations.internal_call import InternalCall
+from slither.slithir.operations.solidity_call import SolidityCall
+from slither.slithir.operations.binary import Binary
+from slither.analyses.data_dependency.data_dependency import is_dependent
+
+
+class PriceManipulationDetector(AbstractDetector):
+ ARGUMENT = 'pess-price-manipulation' # slither will launch the detector with slither.py --detect mydetector
+ HELP = 'Contract math can be manipulated through external token transfers.'
+ IMPACT = DetectorClassification.HIGH
+ CONFIDENCE = DetectorClassification.LOW
+
+ WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/price_manipulation.md'
+ WIKI_TITLE = '# Price Manipulation through token transfers'
+ WIKI_DESCRIPTION = "Calculations could be manipulated through direct transfers to the contract, increasing its balance as they depends on these balances."
+ WIKI_EXPLOIT_SCENARIO = 'N/A'
+ WIKI_RECOMMENDATION = 'Avoid possible manipulations of calculations because of external transfers.'
+
+ def _detect(self) -> List[Output]:
+ all_balance_vars = []
+ all_supply_vars = []
+ all_binary_ops = []
+ for contract in self.contracts:
+ if not contract.is_interface:
+ for func in contract.functions:
+ for n in func.nodes:
+ for x in n.irs:
+ if isinstance(x, SolidityCall):
+ if x.function.name == "balance(address)" or x.function.name == "self.balance" or x.function.name == "this.balance()":
+ all_balance_vars.append((n, x._lvalue))
+ if isinstance(x, HighLevelCall):
+ if str(x.function_name).lower() == "balanceof":
+ all_balance_vars.append((n, x._lvalue))
+ if "supply" in str(x.function_name).lower():
+ all_supply_vars.append((n, x._lvalue))
+ if isinstance(x, InternalCall):
+ if "supply" in str(x.function_name).lower():
+ all_supply_vars.append((n, x._lvalue))
+ if isinstance(x, Binary):
+ all_binary_ops.append((n, x))
+ results = []
+ for (balance_node, bal) in all_balance_vars:
+ for (supply_node, supply) in all_supply_vars:
+ for (node, bin_op) in all_binary_ops:
+ l, r = bin_op.get_variable
+ is_bal_dependent = is_dependent(l, bal, contract)
+ is_supply_dependent = is_dependent(r, supply, contract)
+ if is_bal_dependent and is_supply_dependent:
+ results.append((node, balance_node, supply_node))
+ if not results:
+ return []
+
+
+ response = []
+ for issue_node, balance_node, supply_node in results:
+ res = []
+ res.append("Calculation ")
+ res.append(issue_node)
+ res.append(" depends on balance and token supply - these values could be manipulated through external calls.\n")
+ res.append("\tBalance dependency: ")
+ res.append(balance_node)
+ res.append("\n")
+ res.append("\tSupply dependency: ")
+ res.append(supply_node)
+ res.append("\n")
+ response.append(self.generate_result(res))
+ return response

slitherin/detectors/strange_setter.py

@@ -3,6 +3,8 @@
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
 from slither.core.declarations import Function
 import slither.core.expressions.new_array as na
+import slither.core.expressions.new_contract as nc
+from slither.analyses.data_dependency.data_dependency import is_dependent
 
 
 class StrangeSetter(AbstractDetector):
@@ -51,9 +53,18 @@ def _is_strange_setter(self, fun: Function) -> bool:
  for external in fun.external_calls_as_expressions:
  if isinstance(external._called, na.NewArray):
  continue
+ if isinstance(external._called, nc.NewContract): # skip new contract calls, idk how to get arguments passed to creation
+ continue
  for arg in [*external.arguments, external._called._expression]:
  if str(arg) == str(param):
  used_params.add(param)
+ if fun.name == "constructor":
+ for base_call in fun.explicit_base_constructor_calls:
+ if not self._is_strange_constructor(base_call):
+ for param_cur in fun.parameters:
+ for param_base in base_call.parameters:
+ if is_dependent(param_base, param_cur, base_call):
+ used_params.add(param_cur)
  intersection_len = len(set(fun.parameters) & used_params)
  return intersection_len != len(fun.parameters)
 

tests/price_manipulation_test.sol

@@ -0,0 +1,47 @@
+interface IERC20 {
+ function totalSupply() external view returns (uint256);
+ function balanceOf(address account) external view returns (uint256);
+}
+contract Test1 {
+ IERC20 token;
+
+ function test_vuln_1() external returns(uint256 price) {
+ price = token.balanceOf(address(this)) / token.totalSupply();
+ }
+
+ function test_vuln_2() external returns(uint256 price) {
+ uint256 bal = token.balanceOf(address(this));
+ uint256 supply = token.totalSupply();
+ price = bal / supply; 
+ }
+
+ function test_vuln_3() external returns(uint256 price) {
+ uint256 bal = getBalance();
+ price = bal / token.totalSupply();
+ }
+
+ function test_vuln_4() external returns(uint256 price) {
+ uint256 bal = getBalance();
+ price = 10 + (bal / (token.totalSupply() * 5));
+ }
+
+ function test_vuln_5() external returns(uint256 price) {
+ price = getBalance() / mySupply();
+ }
+
+ function test_vuln_6() external returns(uint256 price) {
+ price = getBalance() + mySupply() + 1;
+ }
+
+ function test_vuln_7() external returns(uint256 price) {
+ price = address(token).balance / mySupply();
+ }
+
+ function getBalance() public returns(uint256 bal) {
+ bal = token.balanceOf(msg.sender);
+ }
+
+ function mySupply() public returns (uint256) {
+ return 100;
+ }
+}

tests/strange_setter_test.sol

@@ -87,3 +87,13 @@ contract OkConstructor {
  init = true;
  }
 }
+
+contract TestInheritance is StrangeSetter{
+ constructor(uint256 _toSet) StrangeSetter(_toSet) {}
+}
+
+contract TestNewContract {
+ constructor(uint256 _toSet) {
+ new TestInheritance(_toSet);
+ }
+}